Skip to content

Commit

Permalink
doc: prepare for v8/V8 linting in doc text
Browse files Browse the repository at this point in the history
PR-URL: #17163
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
  • Loading branch information
Trott authored and MylesBorins committed Dec 12, 2017
1 parent bd1dbce commit 9dcf748
Show file tree
Hide file tree
Showing 13 changed files with 42 additions and 18 deletions.
1 change: 1 addition & 0 deletions doc/api/all.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
<!--lint disable prohibited-strings-->
@include documentation
@include synopsis
@include assert
Expand Down
6 changes: 3 additions & 3 deletions doc/api/process.md
Original file line number Diff line number Diff line change
Expand Up @@ -1459,9 +1459,9 @@ tarball.
* `lts` {string} a string label identifying the [LTS][] label for this release.
This property only exists for LTS releases and is `undefined` for all other
release types, including _Current_ releases. Currently the valid values are:
- `'Argon'` for the v4.x LTS line beginning with v4.2.0.
- `'Boron'` for the v6.x LTS line beginning with v6.9.0.
- `'Carbon'` for the v8.x LTS line beginning with v8.9.1.
- `'Argon'` for the 4.x LTS line beginning with 4.2.0.
- `'Boron'` for the 6.x LTS line beginning with 6.9.0.
- `'Carbon'` for the 8.x LTS line beginning with 8.9.1.

For example:

Expand Down
2 changes: 2 additions & 0 deletions doc/changelogs/CHANGELOG_ARCHIVE.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# Node.js ChangeLog Archive

<!--lint disable prohibited-strings-->

<table>
<tr>
<th>v0.11</th>
Expand Down
2 changes: 2 additions & 0 deletions doc/changelogs/CHANGELOG_IOJS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# io.js ChangeLog

<!--lint disable prohibited-strings-->

<table>
<tr>
<th>v3</th>
Expand Down
2 changes: 2 additions & 0 deletions doc/changelogs/CHANGELOG_V010.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# Node.js 0.10 ChangeLog

<!--lint disable prohibited-strings-->

<table>
<tr>
<th colspan="2">Stable</th>
Expand Down
2 changes: 2 additions & 0 deletions doc/changelogs/CHANGELOG_V012.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# Node.js 0.12 ChangeLog

<!--lint disable prohibited-strings-->

<table>
<tr>
<th>Stable</th>
Expand Down
2 changes: 2 additions & 0 deletions doc/changelogs/CHANGELOG_V4.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# Node.js 4 ChangeLog

<!--lint disable prohibited-strings-->

<table>
<tr>
<th>LTS 'Argon'</th>
Expand Down
2 changes: 2 additions & 0 deletions doc/changelogs/CHANGELOG_V5.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# Node.js 5 ChangeLog

<!--lint disable prohibited-strings-->

<table>
<tr>
<th>Stable</th>
Expand Down
2 changes: 2 additions & 0 deletions doc/changelogs/CHANGELOG_V6.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# Node.js 6 ChangeLog

<!--lint disable prohibited-strings-->

<table>
<tr>
<th>LTS 'Boron'</th>
Expand Down
2 changes: 2 additions & 0 deletions doc/changelogs/CHANGELOG_V7.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# Node.js 7 ChangeLog

<!--lint disable prohibited-strings-->

<table>
<tr>
<th title="Previously called 'Stable'">Current</th>
Expand Down
2 changes: 2 additions & 0 deletions doc/changelogs/CHANGELOG_V8.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# Node.js 8 ChangeLog

<!--lint disable prohibited-strings-->

<table>
<tr>
<th>LTS 'Carbon'</th>
Expand Down
2 changes: 2 additions & 0 deletions doc/changelogs/CHANGELOG_V9.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# Node.js 9 ChangeLog

<!--lint disable prohibited-strings-->

<table>
<tr>
<th>Current</th>
Expand Down
33 changes: 18 additions & 15 deletions doc/guides/maintaining-V8.md
Original file line number Diff line number Diff line change
Expand Up @@ -181,16 +181,16 @@ to be cherry-picked in the Node.js repository and V8-CI must test the change.

* For each abandoned V8 branch corresponding to an LTS branch that is affected by the bug:
* Open a cherry-pick PR on nodejs/node targeting the appropriate *vY.x-staging* branch (e.g. *v6.x-staging* to fix an issue in V8-5.1).
* 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 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.
* Run the Node.js [V8-CI](https://ci.nodejs.org/job/node-test-commit-v8-linux/) in addition to the [Node.js CI](https://ci.nodejs.org/job/node-test-pull-request/).

An example for workflow how to cherry-pick consider the following bug:
https://crbug.com/v8/5199. From the bug we can see that it was merged by V8 into
5.2 and 5.3, and not into V8 5.1 (since it was already abandoned). Since Node.js
`v6.x` uses V8 5.1, the fix needed to cherry-picked. To cherry-pick, here's an
example workflow:
An example for workflow how to cherry-pick consider the bug
[RegExp show inconsistent result with other browsers](https://crbug.com/v8/5199).
From the bug we can see that it was merged by V8 into 5.2 and 5.3, and not into
V8 5.1 (since it was already abandoned). Since Node.js `v6.x` uses V8 5.1, the
fix needed to be cherry-picked. To cherry-pick, here's an example workflow:

* Download and apply the commit linked-to in the issue (in this case a51f429). `curl -L https://github.com/v8/v8/commit/a51f429.patch | git am -3 --directory=deps/v8`. If the branches have diverged significantly, this may not apply cleanly. It may help to try to cherry-pick the merge to the oldest branch that was done upstream in V8. In this example, this would be the patch from the merge to 5.2. The hope is that this would be closer to the V8 5.1, and has a better chance of applying cleanly. If you're stuck, feel free to ping @ofrobots for help.
* Modify the commit message to match the format we use for V8 backports and replace yourself as the author. `git commit --amend --reset-author`. You may want to add extra description if necessary to indicate the impact of the fix on Node.js. In this case the original issue was descriptive enough. Example:
Expand Down Expand Up @@ -289,7 +289,7 @@ 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://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:

```shell
cd $NODE_DIR
Expand All @@ -306,20 +306,23 @@ This should be followed up with manual refloating of all relevant patches.

The fact that Node.js keeps a vendored, potentially edited copy of V8 in deps/
makes the above processes a bit complicated. An alternative proposal would be to
create a fork of V8 at nodejs/v8 that would be used to maintain the V8 branches.
This has several benefits:
create a fork of V8 at `nodejs/v8` that would be used to maintain the V8
branches. This has several benefits:

* The process to update the version of V8 in Node.js could be automated to track the tips of various V8 branches in nodejs/v8.
* It would simplify cherry-picking and porting of fixes between branches as the version bumps in v8-version.h would happen as part of this update instead of on every change.
* The process to update the version of V8 in Node.js could be automated to track
the tips of various V8 branches in `nodejs/v8`.
* It would simplify cherry-picking and porting of fixes between branches as the version bumps in `v8-version.h` would happen as part of this update instead of on every change.
* It would simplify the V8-CI and make it more automatable.
* The history of the V8 branch in nodejs/v8 becomes purer and it would make it easier to pull in the V8 team for help with reviewing.
* The history of the V8 branch in `nodejs/v8` becomes purer and it would make it
easier to pull in the V8 team for help with reviewing.
* It would make it simpler to setup an automated build that tracks Node.js master + V8 lkgr integration build.

This would require some tooling to:

* A script that would update the V8 in a specific Node.js branch with V8 from upstream (dependent on branch abandoned vs. active).
* We need a script to bump V8 version numbers when a new version of V8 is promoted from nodejs/v8 to nodejs/node.
* Enabled the V8-CI build in Jenkins to build from the nodejs/v8 fork.
* We need a script to bump V8 version numbers when a new version of V8 is
promoted from `nodejs/v8` to `nodejs/node`.
* Enabled the V8-CI build in Jenkins to build from the `nodejs/v8` fork.

## Proposal: Dealing with the need to float patches to a stable/beta

Expand All @@ -344,4 +347,4 @@ up working, we will investigate making this change upstream.

<sup>1</sup>Node.js 0.12 and older are intentionally omitted from this document as their support is ending soon.

<sup>2</sup>It seems that @targos is working on port of this script here https://github.com/targos/update-v8.
<sup>2</sup>@targos is working on [a port of this script](https://github.com/targos/update-v8).

0 comments on commit 9dcf748

Please sign in to comment.