[github] Tweak PR template #20381
[github] Tweak PR template #20381
Conversation
| <!-- | ||
| - Features and deprecations should be submitted against the master branch | ||
| - Bug fixes should be submitted against the lowest branch where they apply | ||
| (lowest branches are regularly merged to upper ones so they get the fixes too) |
There was a problem hiding this comment.
we have to give this explanation too often, let's add it upfront
ada9da5 to
995240a
Compare
| @@ -1,11 +1,18 @@ | ||
| <!-- | ||
| - Please edit this template according to the PR you're about to submit | ||
| - Features and deprecations should be submitted against the master branch |
995240a to
605033b
Compare
| - Please fill in this template according to the PR you're about to submit | ||
| - Features and deprecations must be submitted against the master branch | ||
| - Bug fixes must be submitted against the lowest branch where they apply | ||
| (lowest branches are regularly merged to upper ones so they get the fixes too) |
There was a problem hiding this comment.
Can you add full stops at the end of the sentences? I was reading it like one big sentence and it felt off
| | Q | A | ||
| | ------------- | --- | ||
| | Branch? | "master" for new features / 2.7, 2.8 or 3.1 for fixes |
There was a problem hiding this comment.
Would wrapping the placeholders in a comment be relevant, in order to simply have to type value in front of it, instead of removing the whole thing everytime ?
-| Branch? | master / 2.7, 2.8 or 3.1 (see comment)
+| Branch? | <!-- master / 2.7, 2.8 or 3.1 (see comment) -->There was a problem hiding this comment.
Bad idea IMHO: makes more things to remove, both for newcomers and oldcomers. And failling to remove the surrounding <!-- --> will make the field blank. And you know this will happen :)
There was a problem hiding this comment.
No need for removing it actually. Having this as a comment (as for the top comment you added in this PR) allows to simply keep it, and just type the value in front of it.
Having the field blank by mistake is probably not worse than having the full placeholder displayed 😅
(but as for the top comment, it'll appear in the merge commit description :/)
There was a problem hiding this comment.
How about moving the top comment below then ?
There was a problem hiding this comment.
Wild idea: use github's base branch, drop this field and mention the supported branches with a comment...
9916b15 to
1f7b416
Compare
|
I don't like the comments. PR templates should be templates. Contributing guidelines should be described in If it turns out people don't read that doc, simply adding a small link to it seems better than duplicating contributing guidelines in so many places imo. |
that is already proven, and I suggest trying with a comment here instead, see if it makes things better. |
1f7b416 to
cc6e1db
Compare
|
Comment moved to the bottom |
516cd94 to
6f87939
Compare
|
👍 let's see if this improves PR descriptions |
|
👍 let's try it |
|
I like @wouterj suggestion to add a link to the CONTRIBUTOR guide. |
d21d57c to
63bd6d2
Compare
|
The link to contributing is already there in the UI:
|
b867d66 to
06aa298
Compare
06aa298 to
2acaf5f
Compare
|
Thank you @nicolas-grekas. |
This PR was merged into the 2.7 branch. Discussion ---------- [github] Tweak PR template | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Related to #20295 Commits ------- 2acaf5f [github] Tweak PR template
Related to #20295