doc: update backporting guide by refack · Pull Request #13749 · nodejs/node

refack · 2017-06-17T20:32:06Z

also update STYLE_GUIDE comment about emdashes

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
documentation is changed or added
commit message follows commit guidelines

Affected core subsystem(s)

doc

Trott · 2017-06-17T20:34:51Z

doc/STYLE_GUIDE.md

Nit: Perhaps this should link to https://en.wikipedia.org/wiki/Dash#Em_dash instead?

vsemozhetbyt · 2017-06-17T21:39:19Z

doc/guides/backporting-to-release-lines.md

those the commits -> those commits?

gibfahn

Assorted nits (many from the existing text, but while you're here...)

gibfahn · 2017-06-17T22:24:16Z

doc/guides/backporting-to-release-lines.md

I didn't realise this was in the guide, I remember @MylesBorins suggested we remove it in the original PR, as it will get out of date every few months.

As this info is basically duplicated in the LTS schedule, maybe link to that in ## Staging branches and remove the whole ### Active staging branches section.

gibfahn · 2017-06-17T22:24:36Z

doc/guides/backporting-to-release-lines.md

v8.x -> vN.x

gibfahn · 2017-06-17T22:26:00Z

doc/guides/backporting-to-release-lines.md

How about:

(v4.xandv6.x as of March 2017) -> (see the [LTS Plan][]) and then remove Please see the [LTS Plan][] for more information. from the next line.

gibfahn · 2017-06-17T22:30:30Z

doc/guides/backporting-to-release-lines.md

that a backport is needed. -> requesting that a backport pull request be made

gibfahn · 2017-06-17T22:35:45Z

doc/guides/backporting-to-release-lines.md

referance -> reference

NAck. It's an MD feature:

gibfahn · 2017-06-17T22:36:24Z

doc/guides/backporting-to-release-lines.md

I don't think this should be done until the PR lands.

Moved it out of the numbered list, PTAL.

gibfahn · 2017-06-17T22:38:20Z

doc/guides/backporting-to-release-lines.md

All backport PRs are reviewed, what exactly is your meaning here?

I have no Idea, just reformatted from previous text.
Maybe this alludes to the case when the cherry-pick had conflicts that had to be manually resolved.
Or cases like #13750 where I had to add unrelated code i.e. d429bc6

gibfahn · 2017-06-17T22:40:52Z

doc/guides/backporting-to-release-lines.md

This is maybe just me, but I feel like a. b. c. is more natural than another set of numbers.

This still needs doing.

NAck. It's an MD feature:

(pasted this before in the wrong comment)

refack · 2017-06-21T00:20:19Z

@gibfahn PTAL

gibfahn

Mostly done, one nit and one thing I just noticed we don't specify.

gibfahn · 2017-06-21T07:40:54Z

doc/guides/backporting-to-release-lines.md

This still needs doing.

gibfahn · 2017-06-21T07:42:24Z

doc/guides/backporting-to-release-lines.md

While you're here, could you add a note saying that you should run CI as well? Just node-test-pull-request with the default settings.

Refs: #13835 (comment)

refack

CR for PR tile

refack · 2017-06-22T19:06:24Z

doc/guides/backporting-to-release-lines.md

I'm asking to change the format of the titles from parans '(' to brackets '[' for personal reasons:

Green arrow is an issue with new comments
Red arrow is a backport PR

[] seems reasonable, but why require the #PR NUMBER? That seems like noise, especially if you have the commit message in the PR description (the PR-URL is a clickable link so more useful). Also doesn't work for multiple PRs backported together.

Ack. No #PR NUMBER

gibfahn

More nits

gibfahn · 2017-06-22T21:09:32Z

doc/STYLE_GUIDE.md

emdashes -> Em dashes

gibfahn · 2017-06-22T21:11:13Z

doc/guides/backporting-to-release-lines.md

this -> like this

gibfahn · 2017-06-22T21:12:00Z

doc/guides/backporting-to-release-lines.md

[] seems reasonable, but why require the #PR NUMBER? That seems like noise, especially if you have the commit message in the PR description (the PR-URL is a clickable link so more useful). Also doesn't work for multiple PRs backported together.

gibfahn · 2017-06-22T21:14:11Z

doc/guides/backporting-to-release-lines.md

barnch -> branch

gibfahn · 2017-06-22T21:14:25Z

doc/guides/backporting-to-release-lines.md

UNTO -> ONTO

vsemozhetbyt · 2017-06-23T11:20:57Z

doc/guides/backporting-to-release-lines.md

Maybe we better use v6.x in all the examples instead of v7.x? The latter is in the maintenance phase, there will be hardly any backports for it.

😄 just had the same idea!

refack

Addressed

refack · 2017-06-23T11:42:27Z

doc/STYLE_GUIDE.md

refack · 2017-06-23T11:44:06Z

doc/guides/backporting-to-release-lines.md

refack · 2017-06-23T11:44:51Z

doc/guides/backporting-to-release-lines.md

Ack. No #PR NUMBER

refack · 2017-06-23T11:45:11Z

doc/guides/backporting-to-release-lines.md

refack · 2017-06-23T11:45:20Z

doc/guides/backporting-to-release-lines.md

refack · 2017-06-23T11:48:36Z

@gibfahn @vsemozhetbyt PTAL

vsemozhetbyt

But I think this also needs to be reviewed by some collaborators that bear LTS releases.

refack · 2017-06-23T12:41:53Z

@addaleax @MylesBorins You've been requesting backports, PTAL?...

gibfahn

LGTM, thanks for doing this @refack

* also update STYLE_GUIDE comment about Em dashes PR-URL: nodejs#13749 Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>

* also update STYLE_GUIDE comment about Em dashes PR-URL: #13749 Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>

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

Trott reviewed Jun 17, 2017

View reviewed changes

refack force-pushed the backport-guide-learing-by-editing branch from 372f293 to 0312e28 Compare June 17, 2017 20:38

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

vsemozhetbyt reviewed Jun 17, 2017

View reviewed changes

refack mentioned this pull request Jun 17, 2017

doc: adds options for cpp linting to help section of Windows build script #11992

Closed

3 tasks

gibfahn suggested changes Jun 17, 2017

View reviewed changes

gibfahn reviewed Jun 17, 2017

View reviewed changes

refack mentioned this pull request Jun 21, 2017

Question about backporting #13833

Closed

gibfahn reviewed Jun 21, 2017

View reviewed changes

refack commented Jun 22, 2017

View reviewed changes

refack self-assigned this Jun 22, 2017

gibfahn reviewed Jun 22, 2017

View reviewed changes

vsemozhetbyt reviewed Jun 23, 2017

View reviewed changes

refack commented Jun 23, 2017

View reviewed changes

refack force-pushed the backport-guide-learing-by-editing branch from fcb21b9 to e673666 Compare June 23, 2017 11:47

vsemozhetbyt approved these changes Jun 23, 2017

View reviewed changes

gibfahn approved these changes Jun 23, 2017

View reviewed changes

doc: update backporting guide

e3ea0fc

* also update STYLE_GUIDE comment about Em dashes PR-URL: nodejs#13749 Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>

refack force-pushed the backport-guide-learing-by-editing branch from e673666 to e3ea0fc Compare June 23, 2017 19:24

refack merged commit e3ea0fc into nodejs:master Jun 23, 2017

addaleax mentioned this pull request Jun 24, 2017

v8.1.3 proposal #13861

Merged

addaleax pushed a commit that referenced this pull request Jun 24, 2017

doc: update backporting guide

a63e54a

* also update STYLE_GUIDE comment about Em dashes PR-URL: #13749 Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>

addaleax mentioned this pull request Jun 24, 2017

v8.2.0 proposal #13744

Merged

refack deleted the backport-guide-learing-by-editing branch July 2, 2017 02:34

MylesBorins pushed a commit that referenced this pull request Jul 17, 2017

doc: update backporting guide

092bba5

* also update STYLE_GUIDE comment about Em dashes PR-URL: #13749 Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>

MylesBorins mentioned this pull request Jul 18, 2017

v6.11.2 proposal #14356

Merged

refack removed their assignment Oct 20, 2018

Uh oh!

Conversation

refack commented Jun 17, 2017

Checklist

Affected core subsystem(s)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gibfahn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

refack commented Jun 21, 2017

Uh oh!

gibfahn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

refack left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gibfahn left a comment

Choose a reason for hiding this comment

Uh oh!