PR template improvements #14148
PR template improvements #14148
Conversation
sacha-l
added
A2-insubstantial
|
The info about the labels is important though, no? I always find myself having to look at that |
True.. My thinking is that because that information exists in CONTRIBUTING.adoc, point authors to that resource. Happy to add it back if you think its useful to have it in the checklist. |
docs/PULL_REQUEST_TEMPLATE.md
Outdated
| - [ ] My PR includes a detailed description as outlined in the "Description" section above | ||
| - [ ] My PR follows the [labeling requirements](./CONTRIBUTING.adoc#merge-process) of this project (at minimum one label for each `A`, `B`, `C` and `D` required) | ||
| - [ ] My code follows the [style guidelines](./STYLE_GUIDE.md) of this project |
There was a problem hiding this comment.
Why repeat stuff about the description again?
Labeling is enforced by CI and external people can not change lables?
Style guide aka fmt is also checked via CI
There was a problem hiding this comment.
This is a fair point because in theory they shouldn't need to manually check these off since CI will be checking them, but finding out about something only when it fails isn't the best experience imo. Even if they can't change them, it's nice to know about them beforehand
There was a problem hiding this comment.
Maybe we should include a note that outside contributors cannot set the labels directly but that someone will set them when the pr is reviewed
There was a problem hiding this comment.
How about just adding this note under the #Checklist heading:
The CI bot will check that you have fulfilled the labeling and style guide requirements detailed in the checklist below. Don't worry if you don't have permissions to set the labels, you can still submit your PR and the person reviewing will set them for you.
There was a problem hiding this comment.
Why put there redundant information? I mean people will see this ^^
There was a problem hiding this comment.
Yeah I would remove the old style-guide. It pertains to the days that we didn't use rustfmt 🙈
|
bot rebase |
|
Rebased |
There was a problem hiding this comment.
Please link to https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword as well, since I see a lot of people who seemingly are not aware of it.
docs/PULL_REQUEST_TEMPLATE.md
Outdated
| - [ ] My PR includes a detailed description as outlined in the "Description" section above | ||
| - [ ] My PR follows the [labeling requirements](./CONTRIBUTING.adoc#merge-process) of this project (at minimum one label for each `A`, `B`, `C` and `D` required) | ||
| - [ ] My code follows the [style guidelines](./STYLE_GUIDE.md) of this project |
There was a problem hiding this comment.
Yeah I would remove the old style-guide. It pertains to the days that we didn't use rustfmt 🙈
|
bot rebase |
|
Rebased |
|
bot merge |
* Minor edits to CONTRIBUTING * Update PULL_REQUEST_TEMPLATE * Add Closes for GH semantic linking * Update docs/PULL_REQUEST_TEMPLATE.md * Update docs/PULL_REQUEST_TEMPLATE.md * Apply suggestions from code review * Update docs/PULL_REQUEST_TEMPLATE.md --------- Co-authored-by: parity-processbot <>
This PR updates the
PULL_REQUEST_TEMPLATE.mdto make it simpler to digest, with links to the labeling requirements and the style guide instead of duplicating that content. It also places more of a focus on the "Description" section, providing more granular guidelines to motivate authors to submit PRs with clear descriptions.I've also made some minor edits to
CONTRIBUTING.adoc, which I caught while making changes to the PR template.