Repository navigation
doc: add doc on when to backport V8 patches to main - #66710
legendecas wants to merge 4 commits into
Conversation
Signed-off-by: Chengzhong Wu <cwu631@bloomberg.net>
|
Review requested:
|
Signed-off-by: Chengzhong Wu <cwu631@bloomberg.net>
Signed-off-by: Chengzhong Wu <cwu631@bloomberg.net>
| * It fixes a security issue. | ||
|
|
||
| If the patch meet the following criteria, it's recommended to backport the patches | ||
| to the V8 upgrade staging branches, instead of `main`: |
There was a problem hiding this comment.
Are those V8 upgrade staging branches defined somewhere? I for one don’t know what they are 🤔
There was a problem hiding this comment.
It refers to the branch of PRs like #66601. @joyeecheung correct me if I'm having a wrong impression.
There was a problem hiding this comment.
By staging branches, I mean v26.x-staging etc. the release staging branches. Targetting upgrades only complicate them further, unless it's an urgent patch, it's better to simply wait until the base is sufficiently up-to-date before floating a patch at all (IMO we should try not to float any V8 patch on main at all if we can avoid them. It's more reasonable to float V8 patches for release branches if people want them out in releases sooner).
ffdff3c to
6f5b7ee
Compare
Signed-off-by: Chengzhong Wu <cwu631@bloomberg.net>
6f5b7ee to
45007af
Compare
| * It unblocks a pending V8 update, for example by adjusting how the Node.js code | ||
| uses a V8 API. |
There was a problem hiding this comment.
| * It unblocks a pending V8 update, for example by adjusting how the Node.js code | |
| uses a V8 API. | |
| * It unblocks a pending V8 update, for example by adjusting how the Node.js code | |
| uses a V8 API. |
Isn't it the reverse i.e. I think this is talking about the situation where we change src/ or some gyp files, not deps/v8/ but in anticipation of deps/v8 changes, even though this section is supposed to be about changes to deps/v8? If we are trying to cover cases where we are updating deps/v8/ so that we can land some change in src/, I think it generally goes to the second category.
There was a problem hiding this comment.
I was thinking of cases in the past like V8 global proxy interceptor and vm changes, where we have to update both node_contextify, and apply the V8 changes together to avoid breaking behavior in the upcoming V8 upgrade. I limited this to "unblocks a pending V8 update" so that it does not encourage new language features to be individually backported.
|
|
||
| If the patch meet the following criteria, it's recommended to backport the patches | ||
| to the V8 upgrade staging branches, instead of `main`: | ||
| to the release staging branches like `v<MAJOR>.x-staging`, instead of `main`: |
There was a problem hiding this comment.
Do we need to also remind folks that we typically don't accept PRs targeting LTS staging branches?
There was a problem hiding this comment.
Would you like suggest words? I didn't find any docs saying the criteria on backporting dependency patches specifically.
8 participants
As part of our dependency maintenance rules (like
automated dependency updaters for security reasons),
the general rule is "don't backport individual V8 patches".
Increasing floating patches on the
deps/v8further delay the pendingV8 upgrade and increase the cost to land the upgrade in time.
The backport should only be applied to certain categories of patches,
like fixing builds, V8 API usages, security issues.
I intentionally make this rule strict as proposed in #66393.
I believe this makes the criteria more clearer in a first instinct,
without too many mental cost on determine if we should backport or not.
Refs: #66393