Mention labels and milestones in PR review guidelines #15252
Conversation
7e47918 to
e2d22ec
Compare
|
@story645 Thanks, I appreciate your care for the detail of my wording! It's sometimes hard to find the right words as a non-native speaker. Please continue to correct if you see that I'm writing something clumsy or strange. 😄 |
|
No problem, feel free to tag me on your doc PRs! |
doc/devel/coding_guide.rst
Outdated
|
|
||
| * the next bugfix release for bugfixes; e.g. ``v.3.X.Y`` | ||
|
|
||
| Primary PRs should still target the master branch. PRs milestoned for |
There was a problem hiding this comment.
Not sure what you mean here by primary. Are you taking about most PRs? And you don't need the still here.
doc/devel/coding_guide.rst
Outdated
| @@ -47,6 +47,19 @@ PR Review guidelines | ||
| * If you have commit rights, then you are trusted to use them. Please | ||
| help review and merge PRs! | ||
|
|
||
| * Tag the PR with descriptive labels. See the |
There was a problem hiding this comment.
I'm not sure if contributors without triage rights can add labels, even on their own PRs. If they can't, then qualify this statement: If you have triage writes, tag the PR...
84c5d78 to
bbbe169
Compare
|
Almost everytime I've hit merge, something has gone horribly wrong so I was wondering if somebody could walk me through it. |
|
There is no v3.2-doc branch yet, so what should this PR be milestoned to? |
|
Technically, this would go for master. I'll try to remember and trigger a backport once we have a v3.2-doc branch. OTOH, we could also just flag this as v3.2.0 and sneak it into the unpublished 3.2 release. Should be uncritical because it's doc-only. And v3.2-doc would branch from v3.2.0 anyway. |
|
@story645 happy to walk you through. I was just about to say that it's easy, but this again is a bit special because we don't have the target branch yet to which it would usually go. 😏 Will get back once we decide what to do. |
|
So currently this talks about changes to the source code. But there are two other kind of PRs,
and for those it's not entirely clear how to milestone them or which to backport where. |
|
@ImportanceOfBeingErnest good point! Clarified these points. |
88da735 to
f9f3ae6
Compare
f9f3ae6 to
62e1185
Compare
There was a problem hiding this comment.
This is now very clear.
It seems @story645 wanted to use this as learning example for merging, I will leave it to her to hit the green button.
|
This PR itself improves on the guidelines how to review 😄 Nevertheless, here's a condensed checklist of the thing to check before merging:
If all is ok, merge! 😄 If you would like to get more in into approving / merging PRs and would like some guidance/second pair of eyes, feel free to ping me on any PR you would like to take on. I'm happy to help (and maybe we could even improve this reviewers guide as we go along). |
|
@timhoffm I'm thinking what about a followup PR that has that checklist at the top of the reviewers guidelines? Also thanks for all your help! |
…252-on-v3.2.x Backport PR #15252 on branch v3.2.x (Mention labels and milestones in PR review guidelines)
@story645 I was just thinking the same thing when I wrote up the checklist 😄. |
PR Summary
Even though reviewers should know, that information was missing.