-
Notifications
You must be signed in to change notification settings - Fork 113
github: add a Pull Request template #3559
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This is intended to serve as a reminder for contributors, and hopefully should help keeping the documentation and release notes up-to-date. This should also save time for reviewers by automatically asking common questions to contributors.
Ah... nice But overall a decent addition |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinner than 23 lines? Then the line to no template at all gets fuzzy...
I like this, especially the intention to work as a reminder to what PRs should have. (Will copy this for other projects!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In regards to testing...
Is "automated tests" synonymous with unit test code?
Not sure how to use "Tests were run". Assuming all code changes are always verified by developers, this is always "true", right?
As for release notes: not sure how we maintain those.
I think we are referring to unit tests ( integration tests are even better but expecting that for all PRs might be overkill )
which is why I think the 2 questions should be merged About the release notes, right now we don't have a rigid process... but having this in the template is good @minijackson do you mind if I merge the 2 questions and we start using this |
I originally meant the 2 questions as:
I wanted to stay as generic as possible, so that the contributor ask themselves how to test their contribution, instead of just running a command that might not be relevant. But I agree the current wording is confusing… @shroffk feel free to edit if you have a better solution.
@georgweiss there's the changelog but it doesn't seem to have been updated for the last few releases. I personally think it would be good to revive it, and asking the contributor to do it is (in my opinion) a good way of not forgetting things. |
Agree that the template should indicate whether unit tests and/or integration tests have been added. |
I am going to merge this.. since it is a very useful addition |
This is intended to serve as a reminder for contributors, and hopefully should help keeping the documentation and release notes up-to-date.
This should also save time for reviewers by automatically asking common questions to contributors.
I've only included some common generic questions in the checklist. The PR template doesn't start with a title because GitHub automatically prepend the PR template with the commit message body (so the title would be after that).