doc: describe labelling process for backports #12431

addaleax · 2017-04-15T20:08:42Z

Based on discussion from the first backporting team meeting, feedback on the process is welcome!

/cc @nodejs/release @nodejs/lts

mscdex · 2017-04-15T20:15:48Z

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.

vsemozhetbyt · 2017-04-15T20:16:59Z

doc/onboarding-extras.md

+  * Applied to PRs for which a backport PR has been made
+* `lts-watch-v?.x`
+  * Applied to PRs which the LTS working group should consider including in a LTS release
+  * Does not indicate that any specific action, but can be effective as messaging to non-collaborators


Is there something missing here? Like 'Does not indicate that any specific action is needed...'.

@vsemozhetbyt Thanks! I added … will be taken,

addaleax · 2017-04-15T20:22:04Z

@mscdex Right… I’ve changed s/made/merged/ but would still welcome feedback from the releasers group to see if that makes sense.

benjamingr · 2017-04-15T20:57:34Z

doc/onboarding-extras.md

+  * Used by releasers to mark a PR as scheduled for inclusion in an LTS release
+  * Applied to the original PR for clean cherry-picks, to the backport PR otherwise
+* `backport-requested-v?.x`
+  * Used to indicate that a PR needs a manual backport to a branch in order to land on it


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.

@benjamingr you’re right. I clarified this a bit based on your suggestion, ptal

LGTM, thanks.

Trott · 2017-04-16T02:41:20Z

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.)

gibfahn · 2017-04-16T13:46:14Z

@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.

addaleax · 2017-04-16T14:00:08Z

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.

There’s already the link to the Labels section in there, so maybe another link to a subsection of it would be a bit confusing?

would you mind adding a sentence about labels to the newly created backporting guide?

Yup, added a link to the section that’s added here

MylesBorins

Generally LGTM. Small question around if we should change land-on to cherry-picked-to

MylesBorins · 2017-04-17T19:52:56Z

doc/onboarding-extras.md

+* `dont-land-on-v?.x`
+  * For changes that do not apply to a certain release line
+  * Also used when the work of backporting a change outweighs the benefits
+* `land-on-v?.x`


should we consider changing this to cherry-picked-to-v?.x

If its only applied after the picking occurs, and only for cherry-picked (aka "not backported"), then SGTM

I'm good with whatever.

What Sam said, if that's how it'll be used, it's a more descriptive/specific tag, so +1.

MylesBorins

LGTM for now, we can bikeshed labels later

sam-github · 2017-04-24T22:00:18Z

doc/onboarding-extras.md

+
+* `dont-land-on-v?.x`
+  * For changes that do not apply to a certain release line
+  * Also used when the work of backporting a change outweighs the benefits


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.

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).

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

sam-github · 2017-05-15T18:30:30Z

doc/onboarding-extras.md

+* `land-on-v?.x`
+  * Used by releasers to mark a PR as scheduled for inclusion in an LTS release
+  * Applied to the original PR for clean cherry-picks, to the backport PR otherwise
+* `backport-requested-v?.x`


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?

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>

addaleax · 2017-07-23T14:07:37Z

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>

nodejs-github-bot added the doc Issues and PRs related to the documentations. label Apr 15, 2017

mscdex added the meta Issues and PRs related to the general management of the project. label Apr 15, 2017

vsemozhetbyt reviewed Apr 15, 2017

View reviewed changes

addaleax force-pushed the backport-labels branch 2 times, most recently from 3a2d227 to 9f7dd55 Compare April 15, 2017 20:21

benjamingr approved these changes Apr 15, 2017

View reviewed changes

benjamingr reviewed Apr 15, 2017

View reviewed changes

addaleax force-pushed the backport-labels branch from 9f7dd55 to ea37ca8 Compare April 15, 2017 21:00

gibfahn mentioned this pull request Apr 16, 2017

doc: add guide for backporting PRs #11099

Closed

2 tasks

addaleax force-pushed the backport-labels branch 3 times, most recently from 48c7784 to eb858d7 Compare April 16, 2017 13:58

jasnell approved these changes Apr 17, 2017

View reviewed changes

MylesBorins suggested changes Apr 17, 2017

View reviewed changes

addaleax force-pushed the backport-labels branch from eb858d7 to a1cbe9f Compare April 21, 2017 22:43

addaleax mentioned this pull request Apr 21, 2017

v4.8.3 proposal #12499

Merged

MylesBorins approved these changes Apr 24, 2017

View reviewed changes

gibfahn mentioned this pull request Apr 24, 2017

Plan for triaging semver-minor backports in LTS meetings nodejs/Release#204

Closed

sam-github reviewed Apr 24, 2017

View reviewed changes

gibfahn mentioned this pull request May 15, 2017

Backporting to v6.x-staging nodejs/Release#216

Closed

sam-github reviewed May 15, 2017

View reviewed changes

doc: describe labelling process for backports

b440e8e

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>

addaleax force-pushed the backport-labels branch from a1cbe9f to b440e8e Compare July 23, 2017 14:06

addaleax merged commit b440e8e into nodejs:master Jul 23, 2017

addaleax deleted the backport-labels branch July 23, 2017 14:07

addaleax mentioned this pull request Jul 24, 2017

v8.3.0 proposal #14452

Closed

addaleax mentioned this pull request Aug 2, 2017

v8.3.0 proposal #14594

Merged

MylesBorins added the land-on-v6.x label Aug 16, 2017

MylesBorins mentioned this pull request Aug 16, 2017

v6.11.3-proposal #14852

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

doc: describe labelling process for backports #12431

doc: describe labelling process for backports #12431

addaleax commented Apr 15, 2017

mscdex commented Apr 15, 2017

vsemozhetbyt Apr 15, 2017

addaleax Apr 15, 2017

addaleax commented Apr 15, 2017

benjamingr Apr 15, 2017

addaleax Apr 15, 2017

benjamingr Apr 15, 2017

Trott commented Apr 16, 2017

gibfahn commented Apr 16, 2017

addaleax commented Apr 16, 2017

MylesBorins left a comment

MylesBorins Apr 17, 2017

sam-github Apr 17, 2017

jasnell Apr 19, 2017

gibfahn Apr 19, 2017

MylesBorins left a comment

sam-github Apr 24, 2017

sam-github May 15, 2017

MylesBorins May 15, 2017

sam-github May 15, 2017

sam-github May 15, 2017

addaleax commented Jul 23, 2017

doc: describe labelling process for backports #12431

doc: describe labelling process for backports #12431

Conversation

addaleax commented Apr 15, 2017

mscdex commented Apr 15, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

addaleax commented Apr 15, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Trott commented Apr 16, 2017

gibfahn commented Apr 16, 2017

addaleax commented Apr 16, 2017

MylesBorins left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MylesBorins left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

addaleax commented Jul 23, 2017