Skip to content

doc: add guide for backporting PRs #11099

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.

Sign up for GitHub

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

Closed
evanlucas wants to merge 11 commits into from
Closed

doc: add guide for backporting PRs #11099

Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
f153855
doc: add guide for backporting prs
evanlucas Feb 1, 2017
837bcb9
fixup! commit message
evanlucas Feb 4, 2017
aa40433
fixup! cherry-pick instructions
evanlucas Feb 4, 2017
444359c
fixup add backport metadata line
evanlucas Feb 13, 2017
066e617
fixup! add details regarding Backport-of
evanlucas Feb 15, 2017
b06d19f
fixup! fix lts plan reference
evanlucas Feb 17, 2017
4db2488
fixup! remove more releaser info
evanlucas Feb 24, 2017
2782890
fixup! more changes
evanlucas Mar 17, 2017
89e7bfb
fixup! use cherry-pick --continue and fix wording
evanlucas Mar 17, 2017
fc434b2
fixup! use original PR-URL
evanlucas Mar 27, 2017
a374208
squash!: remove commit message changes
gibfahn Apr 12, 2017
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
Prev Previous commit
Next Next commit
fixup! add details regarding Backport-of
  • Loading branch information
@evanlucas
evanlucas committed Feb 15, 2017
commit 066e61728b2729562598a8938599854b59721670
31 changes: 28 additions & 3 deletions doc/guides/backporting-to-release-lines.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,34 @@ hint: and commit the result with 'git commit'
* Push the changes to your fork and open a pull request.

@gibfahn gibfahn Mar 12, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There should probably be a:

  • Make sure make -j4 test still passes.

above this line, see #11797 (comment)

* Be sure to target the `v7.x-staging` branch in the pull request.
* When landing a backport commit, please include the PR-URL from the original

@MylesBorins MylesBorins Feb 24, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

/cc @nodejs/lts is everyone on board with this meta data change.

For the record we have not been doing this at all. I for one would like to see this come up at an LTS meeting before we commit to taking on this process. If not in a meeting I would at least like to head from @jasnell @rvagg @mhdawson and @Fishrock123 in this comment thread

@jasnell jasnell Feb 24, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Including the link to the original PR is fine but if this a backport-PR then a different metadata label should be used to avoid accidentally confusing tooling. For instance:

Original-PR-URL: https://... (original PR)
PR-URL: https://... (backport PR)

@gibfahn gibfahn Feb 24, 2017 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@jasnell After the previous discussion in this PR, the consensus for the name (so far) was Backport-of: <origin PR-URL>, it's on the next line.

See #11099 (comment)

@jasnell jasnell Feb 24, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @gibfahn ... missed that earlier discussion

@evanlucas evanlucas Feb 24, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@jasnell that being said, are you alright with that approach?

@jasnell jasnell Feb 24, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, that definitely works.

@mhdawson mhdawson Feb 24, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I good with the approach I think including the link to the original PR in Backport-of: is the right thing to do.

pull request and add `Backport: <backport PR-URL>` to the commit metadata.
The `Backport` line should be directly after the `PR-URL` line in the
metadata.
pull request as `Backport-of: <origin PR-URL>`. The `Backport-of` line should
be directly after the `PR-URL` line in the metadata.

Below are examples of an original commit message and a backport commit message.

In this example, https://github.com/nodejs/node/pull/1234 is the original pull
request and https://github.com/nodejs/node/pull/5678 is the backport.

@vsemozhetbyt vsemozhetbyt Mar 8, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These URL dummies become real URLs and get to be confusing as absolutely different PRs.

@targos targos Mar 29, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@gibfahn my other comment was because of this. If the PR-URL is correct, then this sentence is a bit confusing because PR 5678 doesn't appear in the example


Original:

```
lib: make something faster

Switch to using String#repeat to improve performance.

PR-URL: https://github.com/nodejs/node/pull/1234
```

Backport:

```
lib: make something faster

Switch to using String#repeat to improve performance.

PR-URL: https://github.com/nodejs/node/pull/5678

@MylesBorins MylesBorins Mar 27, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is not accurate. we maintain the original PR-URL

Backport-of: https://github.com/nodejs/node/pull/1234
```

### Helpful Hints

Expand Down