doc: describe labelling process for backports#12431
Conversation
|
The description for 'backported-to-v?.x' seems a little confusing. It currently makes it sound as if it's added simply when the backport PR is made and not necessarily when it has been merged. I would have thought the latter would have been the case for that label, otherwise 'backported' seems misleading. |
doc/onboarding-extras.md
Outdated
There was a problem hiding this comment.
Is there something missing here? Like 'Does not indicate that any specific action is needed...'.
There was a problem hiding this comment.
@vsemozhetbyt Thanks! I added … will be taken,
3a2d227 to
9f7dd55
Compare
|
@mscdex Right… I’ve changed s/made/merged/ but would still welcome feedback from the releasers group to see if that makes sense. |
doc/onboarding-extras.md
Outdated
There was a problem hiding this comment.
in order to land it on it? Although I'd be more explicit without anaphoras and say "in order to land the PR on that branch" or something like that.
We might want to indicate this is typically because the commit doesn't land cleanly on the branch although that's implied.
There was a problem hiding this comment.
@benjamingr you’re right. I clarified this a bit based on your suggestion, ptal
9f7dd55 to
ea37ca8
Compare
|
I'm fine with this in onboarding-extras.md, but if you wanted to put a link in onboarding.md or even just put it directly into onboarding.md, I'd be 👍 to that. (I suspect onboarding-extras should eventually be merged into onboarding.md anyway. It's a short document and I'm not sure I really understand why it ought to be separate.) |
|
@addaleax would you mind adding a sentence about labels to the newly created backporting guide? See #11099 (comment), I think it makes sense to do it in this PR. |
48c7784 to
eb858d7
Compare
There’s already the link to the
Yup, added a link to the section that’s added here |
MylesBorins
left a comment
There was a problem hiding this comment.
Generally LGTM. Small question around if we should change land-on to cherry-picked-to
doc/onboarding-extras.md
Outdated
There was a problem hiding this comment.
should we consider changing this to cherry-picked-to-v?.x
There was a problem hiding this comment.
If its only applied after the picking occurs, and only for cherry-picked (aka "not backported"), then SGTM
There was a problem hiding this comment.
What Sam said, if that's how it'll be used, it's a more descriptive/specific tag, so +1.
eb858d7 to
a1cbe9f
Compare
MylesBorins
left a comment
There was a problem hiding this comment.
LGTM for now, we can bikeshed labels later
doc/onboarding-extras.md
Outdated
There was a problem hiding this comment.
Is this exclusive with backport-requested-to? I thought it would be on a PR that has backport-requested-to, because the original PR cannot land, but @MylesBorins said it should not - I guess because the PR is landing in the abstract, it just needs to be backported to make that happen. Nees to be more clear.
There was a problem hiding this comment.
ping @MylesBorins @nodejs/lts @gib and I are going through nodejs/Release#216 now and hitting this issue. I think backport-requested-to PRs should have been omitted from the gist/issues in #216 above (or we should add dont-land-on labels to them, because dont-land-on get omitted).
There was a problem hiding this comment.
you can filter based on any tag. Don't land should only apply to prs we do not want to land. If a backport is requested add appropriate tag
doc/onboarding-extras.md
Outdated
There was a problem hiding this comment.
Should we apply it to things, like a test change, that we would be willing to land if had a backport?
Or reserve it for things like bug fixes that we actually want to be backported, but that aren't landing?
There was a problem hiding this comment.
for example: #12599 would be worth having on 6.x, just to keep the code synced, but if the author doesn't want to backport, that's OK
Based on discussion from the first backporting team meeting. PR-URL: nodejs#12431 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com>
a1cbe9f to
b440e8e
Compare
|
Landed in b440e8e. Sorry I didn’t just do that earlier, but yes, like Myles said, if you want to continue bikeshedding about this just make your own PRs. |
Based on discussion from the first backporting team meeting. PR-URL: #12431 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com>
Based on discussion from the first backporting team meeting. PR-URL: #12431 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com>
Based on discussion from the first backporting team meeting. PR-URL: #12431 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com>
Based on discussion from the first backporting team meeting. PR-URL: #12431 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com>
Based on discussion from the first backporting team meeting. PR-URL: #12431 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com>
Based on discussion from the first backporting team meeting, feedback on the process is welcome!
/cc @nodejs/release @nodejs/lts