Skip to content

doc: describe labelling process for backports #12431

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 23, 2017
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 27 additions & 4 deletions doc/onboarding-extras.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,33 @@ Please use these when possible / appropriate
git checkout $(git show -s --pretty='%T' $(git show-ref -d $(git describe --abbrev=0) | tail -n1 | awk '{print $1}')) -- test; make -j4 test
```

### LTS/Version labels

We use labels to keep track of which branches a commit should land on:

* `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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

* `land-on-v?.x`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm good with whatever.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

* 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`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

* Used to indicate that a PR needs a manual backport to a branch in order to land the changes on that branch
* Typically applied by a releaser when the PR does not apply cleanly or it breaks the tests after applying
* Will be replaced by either `dont-land-on-v?.x` or `backported-to-v?.x`
* `backported-to-v?.x`
* Applied to PRs for which a backport PR has been merged
* `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 will be taken, but can be effective as messaging to non-collaborators
* `lts-agenda`
* For things that need discussion by the LTS working group
* (for example semver-minor changes that need or should go into an LTS release)
* `v?.x`
* Automatically applied to changes that do not target `master` but rather the `v?.x-staging` branch

Once a release line enters maintenance mode, the corresponding labels do not
need to be attached anymore, as only important bugfixes will be included.

### Other Labels

Expand All @@ -89,10 +116,6 @@ Please use these when possible / appropriate
* Architecture labels
* `arm`, `mips`, `s390`, `ppc`
* No x86{_64}, since that is the implied default
* `lts-agenda`, `lts-watch-v*`
* tag things that should be discussed to go into LTS or should go into a specific LTS branch
* (usually only semver-patch things)
* will come more naturally over time


## Updating Node.js from Upstream
Expand Down