Update PULL_REQUEST_TEMPLATE.md #26412
Conversation
Standardize the Pull Request to include Description & issues fixed Signed-off-by: nishasaini <[email protected]>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: beingnishas The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
As stated in the contributing guidelines, if a PR fixes any issue it should be marked in the PR. This commit adds a `Fixes:` section in the PR template Signed-off-by: nishasaini <[email protected]>
|
[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore. |
| - Sign your commits with your real name. You can amend previous commits with `git commit -s --amend`. | ||
| --> | ||
|
|
||
| #### Description |
There was a problem hiding this comment.
I don't see the point of the header, the way github works it copies the commit description by default as first thing in the field so now you would require me to copy paste my entire commit description each time to below that header. Overall I don't see how this title improves anything so I rather just not have it at all.
| --> | ||
|
|
||
| #### Description | ||
| <!-- Provide a clear summary of your changes and why they are needed. --> |
There was a problem hiding this comment.
This should be the commit message ideally. Unless you send many commits in one PR when it can be a smaller description of the full changes being made.
| Finally, be sure to sign commits with your real name. Since by opening | ||
| a PR you already have commits, you can add signatures if needed with | ||
| something like `git commit -s --amend`. | ||
| Fixes: |
There was a problem hiding this comment.
I rather not have that by default printed. Many PRs don't fix issues at people will not remove this resulting in me thinking they might made a mistake and forgot to copy the issue number there.
There was a problem hiding this comment.
Hi Paul,
Thanks for reviewing. The reason for adding this line specifically is to ensure the issues are added in a certain format. The CONTRIBUTING.md mentions to include "When your PR fixes an issue, please note that by including Fixes: #00000 in the commit description" but still some PRs miss this in commit message. So if we can add something in the PR description itself it will ensure we have issue links.
There was a problem hiding this comment.
Well sure but many changes don't have issues so including this by default make my life harder as I know get a bunch of PR descriptions with empty Fixes: lines. As far I as I can observe a majority of contributors does not read the description so I don't think this alone solves that. The comment below already shows what format to use.
| - If it fixes multiple issues, use commas: | ||
| Fixes: #00000, #00001, #0002 | ||
| Fixes: https://github.com/containers/common/issues/00000, https://github.com/containers/common/issues/00001 | ||
| Fixes: https://issues.redhat.com/browse/RHEL-00000, https://issues.redhat.com/browse/RHEL-00001 | ||
| Fixes: RHEL-00000, RHEL-00001 |
There was a problem hiding this comment.
I don't think that makes sense in terms of the style, one Fixes should only contain one link. If you link multiple they should be on separate lines with Fixes: each. But CONTRIBUTING.md doesn't seem to specify that.
There was a problem hiding this comment.
You are right it is not mentioned in CONTRIBUTING.md, I referred this comment to use multiple fixes separated by comma.
There was a problem hiding this comment.
I don't think it is particular defined anywhere, but IMO comma separated is less readable especially with long links so I rather promote using one per line
| ```release-note | ||
|
|
||
| None |
There was a problem hiding this comment.
Please don't set a default none here, release messages should require an explicit thought. Most people just skip this and then CI should fail and say the message is needed and not assume it doesn't need one as default
|
Also please squash your commits |
| ```release-note | ||
|
|
||
| None |
There was a problem hiding this comment.
I agree. If we set to none, that will be the last meaningful release note helper we get.
This PR standardizes the Pull Request template to include Description & issues fixed. As per the contribution guidelines in here when a PR fixes any issue, the commit description should include
Fixes: #00000but that is not being used in many PRs.To prevent such PRs in future, the template will now provide a new section for "Description and Fixes(if it fixes any issues)"
Does this PR introduce a user-facing change?