Skip to content
This repository has been archived by the owner on Apr 17, 2023. It is now read-only.

doc: add license, issue & PR templates, contributor guides, codeowners #11

Merged
merged 12 commits into from
Dec 7, 2018

Conversation

darahayes
Copy link
Contributor

@darahayes darahayes commented Dec 6, 2018

This PR mostly adds the boilerplate kind of documentation you would expect.

  • License
  • Code of Conduct
  • Multiple Issue Templates (bug report, feature request, ask for help)
  • Pull Request Template
  • Code Owners
  • Top level contributing guide that links to various different guides
  • Various guides for opening issues, PRs, examples etc.
  • Tidies up the Readme to point to other docs.

There is one major feature of this PR. It introduces a commit message guideline. I ask everyone that reviews this PR to please take a close look at these guidelines added in the Pull Requests guide.

Overall, the commit message guidelines are very simple to follow, and they provide a lot of value.

  • Commit messages are more readable, especially when looking through the project history.
  • Commit messages to describe whether a major, minor or patch change has been introduced (see semver.org).
  • Commit messages can be used to generate a changelog.

Copy link

@camilamacedo86 camilamacedo86 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The GitHub files should come from the standards definition.(https://github.com/aerogear/standards/tree/master/.github)

Note that the env here in the ISSUE Template may should be customized according to the context of the project, however, the rest will be all the same.

Copy link
Contributor

@wtrocki wtrocki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pure masterpiece. There is no way to verify this without merge.
Any rules can be adjusted as we go.

@wei-lee
Copy link
Contributor

wei-lee commented Dec 6, 2018

@camilamacedo86 I think the standard repo should be optional, rather than enforced on other repos/projects, even if in the same org. We can keep the code of conduct, and maybe license files, but I do think the PR template and issue template aren't great for every other project. I will create an issue there.

@darahayes
Copy link
Contributor Author

darahayes commented Dec 6, 2018

@wei-lee @camilamacedo86 just to add to that. The CODE_OF_CONDUCT and LICENSE are the exact same ones from the standards repo. This .github folder is designed for metadata that makes the user experience in GitHub better. Things like Issue and PR templates, it happens to support the concept of adding a LICENSE and CODE_OF_CONDUCT there but in my opinion we shouldn't be doing it. The license and code of conduct is not something specific to GitHub and therefore does not belong in the .github directory. I made the decision to place them at the root of the project because that has always been the common convention for open source projects.

@camilamacedo86 I also think that outright rejecting this PR without genuinely reviewing the contents because it doesn't use the templates in the standards repo was not the right thing to do. Instead, we should be asking some of the following questions:

  • Why did someone feel compelled to not use the templates provided by the standards repo?
  • Is the standards repo missing anything? For example, individual issue templates for bug reports, feature requests, asking for help. These are really nice to have.
  • Given our vast array of projects all with different intentions and levels of technicality, is it actually a good idea to have one set of templates that they all must use? For example: if I wanted to open an issue on the standards repo or on the mobile-docs repo, the headings around Expected Behaviour and Actual Behaviour make no sense at all because the repos have nothing to do with code.

@camilamacedo86
Copy link

camilamacedo86 commented Dec 6, 2018

Hi @darahayes,

I agree 100% with your comment regards the location of the LICENSE and CODE_OF_CONDUCT files and we need to change it and make clear in the standards repo as well. Maybe create another dir with this files as the CONTRIBUTION.md as well. Regards the templates I made comments which I hope clarifies my position here.

@@ -0,0 +1,23 @@
---
name: "\U0001F41B Bug report"
Copy link

@camilamacedo86 camilamacedo86 Dec 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is expected in the name and about? It really hard gets the required info from who open the issue. Why not follow up the template here? What is the advantage replace the default sections for the name and the open question about?

Description

Add a brief and meaningful description.

Expected Behavior

Describe the expected behaviour.

Actual Behavior

Describe the current/actual behaviour.

Environment

  • Operating system:
  • OpenShift version:
  • Project Version:

Steps to reproduce


You can also reach the aerogear team at [#aerogear](ircs://chat.freenode.net:6697/aerogear) on [Freenode IRC](https://freenode.net/) or the
[aerogear google group](https://groups.google.com/forum/#!forum/aerogear).

Copy link

@camilamacedo86 camilamacedo86 Dec 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO it is a bug and/or RFE and/or a question should be defined by a label/tag. Users/contributors/QA are not usually able to do it. It is common they believe that an RFE is an issue for example. Do not you think that the field "about:" is something too generic? How expected have here the info which are looking for since it is an open question?

@camilamacedo86
Copy link

camilamacedo86 commented Dec 6, 2018

Hi @wei-lee,

In my point of view, following the same standard for the templates across the teams and projects bring to us many advantages as making easier to work with, and be more productive since we already know what is expected, and what to check, and etc... Also, we could check that the review and the process to address the issues are indeed faster and more productive too.

Probably the best approach to shows how the standards templates have been helpful and its sections are important is show some practical examples as follows.

Now you can check how hard for don't say sometimes impossible is to know WHAT, and WHY, and HOW, and the STEPS and if the PR was or not tested by the owner and other person as what exacly is the issue reported in some following examples without this template/infos

How can anyone review the above PRs or understand what is required to do to address in the opened issues? How to known (WHAT/WHY/HOW/STEPS) in the case we are looking for to found when and why some issue was introduced?

Following some questions/concerns about this PR which may give a better understanding.

  1. The templates here are not suggesting the people fill what we already checked that is required in most of part of the cases.
  2. It has 3 different templates for the issue. How it will works? What template GitHub will show for the contributor?
  3. Is not the SO used and the OCP version envs that could cause different behaviours then why these questions are not required here by default?
  4. Why the name is a relevant field for issue templates? What exactly is expected in this field? How it can be a helpful information for us to address then?
  5. Do not you think that the field "about:" is something too generic? How expected have here the info which are looking for since it is an open question?
  6. Have someone tried to fill these templates and think about how it usually will be filled for all people indeed that ones which not work in the product/team/redhat?

Also, for me, this subject as it advantages is something discussed and decided by ML thread and the community Aerogear meetings already. Everybody had for more than one month the opportunity to contribute with and give your opinion. Why not follow it now?

c/c @darahayes @wtrocki @StephenCoady

@@ -0,0 +1,20 @@
<!--
Copy link

@camilamacedo86 camilamacedo86 Dec 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I liked some points in the Checklist and it could be added as a customization for this repo. Following some argumentations in favor of the sections defined in the PR Template

  • The biggest part of the PR without this template has not the info WHAT, and WHY, and HOW and, and STEPS and just move forward because are reviewed for par team-mate who have the context.
  • If an issue be faced this information is helpful in order to investigate the changes performed in the PRs merged which could introduce the issue
  • We need to keep in mind that our goal is to build a community and receive PRs from people which are not working with us and not attend our meetings and etc..
  • The sections make clear what is expected
  • The template makes clear that is expected direct and short answers and not that big descriptions found when the template is not applied which usually do not contain the required data.
  • The PR owner will revalidate the PR when it will be filled which will be useful to find issues and/or changes that need be done before asking the review since the suggested questions will make the owner think more about it to clarify the achievement and steps to test it.
  • The field to check that the PR was tested by the owner and reviewer push the people to do the tests and avoid we merge issues.

@darahayes
Copy link
Contributor Author

darahayes commented Dec 7, 2018

Hey, sorry I did not get the chance to come back to this until now. Firstly, please see this screenshot (from the Node.js repo) that shows how multiple issue templates work.

screenshot 2018-12-07 at 10 19 30

Is not the SO used and the OCP version envs that could cause different behaviours then why these questions are not required here by default?

This repo is a framework of individual npm libraries that a user imports into their Node.js application. None of them have any interaction with Kubernetes/Openshift in any shape or form. It would be like asking for kubernetes version in a repo like Express. I don't think the information is relevant right now and it could potentially confuse or misdirect potential contributors.

Why the name is a relevant field for issue templates? What exactly is expected in this field? How it can be a helpful information for us to address then?

The name is very relevant. This repo is a monorepo with several individually published packages. See the packages folder. The name helps us identify which module the issue references.

I completely understand the feedback around having sections like Description, Motiviation, What, Why, Verification Steps etc. I do think they are a great idea but I also think that they try to solve problems that we don't actually have right now because we do not have a high volume of people reporting issues or creating Pull Requests. The main intention here is to provide something very simple and user friendly that reduces the barrier to entry when it comes to making a first contribution. I would first like to see us actually having problems with these templates before deciding to improve them.

@aliok
Copy link
Member

aliok commented Dec 7, 2018

I like what @darahayes provided and I also like @camilamacedo86 's efforts to standardize things.
So, can we just not use the standard as much as possible but just customize as we like?

IMO, those standards were more for projects that had nothing and I really appreciate @camilamacedo86 's efforts.
But, obviously standards cannot cover every special case. In this case, @darahayes wants to have more info which is +1 for me.

@camilamacedo86
Copy link

camilamacedo86 commented Dec 7, 2018

Hi @darahayes and @aliok,

Following some points.

  • Really nice to know that is possible have more them one template. Go forward to that and also we could to do the same with the standards to apply for all projects.
  • Off course it is possible to customize them for each project and still following the same standard/nomenclature/structure and etc ..
  • Could the issue templates have not the same structure? I mean current behaviour and expected one, an environment where will be asked the "name"(module of X will make more clear what is expected IHMO)
  • Could we not add open questions? Open questions are not good at all becuase it do not make clear what is expected. (E.g the "about" instead of current behaviour/expected behaviours/steps to reproduce)
  • Regards your comment "I completely understand the feedback around having sections like Description, Motiviation, What, Why, Verification Steps etc. I do think they are a great idea but I also think that they try to solve problems that we don't actually ". The problems that they are solving are that ones which we can check in the PR examples added here which are faced in 90% of the cases where the templates are not followed in all projects. PS.: it is not exactly the sections they are "Description/What/Why/How/Steps/Progress/Checklist/Extra Info"

So, folks. It is just my opinion. Please, feel free to follow as you wish.

@camilamacedo86 camilamacedo86 dismissed their stale review December 7, 2018 15:50

I will dismiss my review.

@darahayes
Copy link
Contributor Author

@camilamacedo86 and thanks for your feedback. While sometimes we might disagree on things, I genuinely appreciate getting those challenges. Open source at its finest 😃

@darahayes darahayes merged commit aa40dc8 into master Dec 7, 2018
@darahayes darahayes deleted the repo-setup branch December 7, 2018 15:59
@camilamacedo86
Copy link

@darahayes really tks for your attention and the mind open. I also discovered really interesting things.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants