Skip to content

doc: include V8 commit URL in V8 backport guide#16054

Closed
gibfahn wants to merge 1 commit into
nodejs:masterfrom
gibfahn:v8-guide
Closed

doc: include V8 commit URL in V8 backport guide#16054
gibfahn wants to merge 1 commit into
nodejs:masterfrom
gibfahn:v8-guide

Conversation

@gibfahn
Copy link
Copy Markdown
Member

@gibfahn gibfahn commented Oct 6, 2017

Use Commit: for the V8 commit, and PR-URL: for the Node PR URL.

Bikeshedding over Commit: welcome.

Refs: #16053 (comment)

Checklist
Affected core subsystem(s)

doc, v8

cc/ @nodejs/v8

@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Oct 6, 2017
Comment thread doc/guides/maintaining-V8.md Outdated
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.

Should this be Refs:?

brb getting more paint

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

SGTM. Surprised you didn't ask for Ref: though...

@targos
Copy link
Copy Markdown
Member

targos commented Oct 6, 2017

I'm not sure we need that. You can already look at the changes at the Reviewed-on: URL that is always present in the original commit message.

Use `Commit:` for the V8 commit, and `PR-URL:` for the Node PR URL.

Refs: nodejs#16053 (comment)
@gibfahn
Copy link
Copy Markdown
Member Author

gibfahn commented Oct 6, 2017

I'm not sure we need that. You can already look at the changes at the Reviewed-on: URL that is always present in the original commit message.

True, but that requires understanding of the Chromium pull request UI, which I find pretty opaque compared to the Github equivalent.

@targos
Copy link
Copy Markdown
Member

targos commented Oct 6, 2017

Fair enough. I'm not against this change if it can help people.

Note to self: update https://github.com/targos/update-v8 when this has landed.

@joyeecheung
Copy link
Copy Markdown
Member

Landed in 9f1e6e7, thanks!

joyeecheung pushed a commit that referenced this pull request Oct 13, 2017
Use `Commit:` for the V8 commit, and `PR-URL:` for the Node PR URL.

Refs: #16053 (comment)
PR-URL: #16054
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
@gibfahn gibfahn deleted the v8-guide branch October 13, 2017 13:27
addaleax pushed a commit to ayojs/ayo that referenced this pull request Oct 15, 2017
Use `Commit:` for the V8 commit, and `PR-URL:` for the Node PR URL.

Refs: nodejs/node#16053 (comment)
PR-URL: nodejs/node#16054
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Oct 18, 2017
Use `Commit:` for the V8 commit, and `PR-URL:` for the Node PR URL.

Refs: #16053 (comment)
PR-URL: #16054
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos added a commit to targos/update-v8 that referenced this pull request Oct 18, 2017
MylesBorins pushed a commit that referenced this pull request Nov 16, 2017
Use `Commit:` for the V8 commit, and `PR-URL:` for the Node PR URL.

Refs: #16053 (comment)
PR-URL: #16054
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Nov 21, 2017
MylesBorins pushed a commit that referenced this pull request Nov 21, 2017
Use `Commit:` for the V8 commit, and `PR-URL:` for the Node PR URL.

Refs: #16053 (comment)
PR-URL: #16054
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Nov 28, 2017
Use `Commit:` for the V8 commit, and `PR-URL:` for the Node PR URL.

Refs: #16053 (comment)
PR-URL: #16054
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc Issues and PRs related to the documentations.

9 participants