doc: update maintainting V8 guide #17260
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
apapirovski
approved these changes
Nov 22, 2017
doc/guides/maintaining-V8.md
Outdated
| * On Node.js < 9.0.0: Increase the patch level version in `v8-version.h`. This will not cause any problems with versioning because V8 will not publish other patches for this branch, so Node.js can effectively bump the patch version. | ||
| * On Node.js >= 9.0.0: Increase the `v8_embedder_string` number in `common.gypi`. | ||
| * In some cases the patch may require extra effort to merge in case V8 has changed substantially. For important issues we may be able to lean on the V8 team to get help with reimplementing the patch. | ||
| * Open a cherry-pick PR on nodejs/node targeting the *vY.x-staging* branch and notify the @nodejs/v8 team. |
There was a problem hiding this comment.
I think this should have backticks around nodejs/node, to follow the rest of the doc.
doc/guides/maintaining-V8.md
Outdated
| @@ -289,17 +294,13 @@ To audit for floating patches: | |||
| git log --oneline deps/v8 | |||
| ``` | |||
|
|
|||
| To replace the copy of V8 in Node.js, use the `[update-v8](https://gist.github.com/targos/8da405e96e98fdff01a395bed365b816)` script<sup>2</sup>. For example, if you want to replace the copy of V8 in Node.js with the branch-head for V8 5.1 branch: | |||
| To replace the copy of V8 in Node.js, use the [`update-v8`](https://github.com/targos/update-v8) tool. For example, if you want to replace the copy of V8 in Node.js with the branch-head for V8 5.1 branch: | |||
There was a problem hiding this comment.
Split to 80 char line lengths while you're here?
There was a problem hiding this comment.
done. while I was there, I split the entire document to 80 char line lengths and put the links at the bottom.
joyeecheung
approved these changes
Nov 23, 2017
doc/guides/maintaining-V8.md
Outdated
| team to get help with reimplementing the patch. | ||
| * Open a cherry-pick PR on `nodejs/node` targeting the *vY.x-staging* branch | ||
| and notify the `@nodejs/v8` team. | ||
| * Run the Node.js [V8 CI] in addition to the [Node.js CI]. |
There was a problem hiding this comment.
Can you mention the test-v8 target in the Makefile (and possibly make-v8.sh)? It takes quite long to actually run locally but mentioning it here at least leaves a pointer about how to update the CI.
There was a problem hiding this comment.
I can. Do you have a suggestion for the wording?
There was a problem hiding this comment.
Maybe Note: The CI uses the `test-v8` target in the `Makefile`, which uses `tools/make-v8.sh` to reconstruct a git tree in the `deps/v8` directory to run V8 tests.
- Mention the now existing Node.js canary branch. - Clearer steps for backporting commits. - Mention that `update-v8` can be used to simplify backports and updates. - Remove section about proposal for floating patches: embedder string has been implemented. - Wrap lines at 80 characters.
targos
added a commit
that referenced
this pull request
Nov 26, 2017
- Mention the now existing Node.js canary branch. - Clearer steps for backporting commits. - Mention that `update-v8` can be used to simplify backports and updates. - Remove section about proposal for floating patches: embedder string has been implemented. - Wrap lines at 80 characters. PR-URL: #17260 Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
|
Landed in 1b9915f |
MylesBorins
pushed a commit
that referenced
this pull request
Dec 12, 2017
- Mention the now existing Node.js canary branch. - Clearer steps for backporting commits. - Mention that `update-v8` can be used to simplify backports and updates. - Remove section about proposal for floating patches: embedder string has been implemented. - Wrap lines at 80 characters. PR-URL: #17260 Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
MylesBorins
pushed a commit
that referenced
this pull request
Dec 12, 2017
- Mention the now existing Node.js canary branch. - Clearer steps for backporting commits. - Mention that `update-v8` can be used to simplify backports and updates. - Remove section about proposal for floating patches: embedder string has been implemented. - Wrap lines at 80 characters. PR-URL: #17260 Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
Merged
|
Don't think this is worth backporting, but if anyone disagrees feel free to raise backports for v6.x and v8.x. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
update-v8can be used to simplify backports andupdates.
has been implemented.