nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
106.63k stars 29.07k forks source link

Node.js 12 SemVer Major Cut-off #26844

Closed BethGriggs closed 5 years ago

BethGriggs commented 5 years ago

/cc @nodejs/tsc @nodejs/release

Refs: https://github.com/nodejs/Release/issues/417

PSA: SemVer Major Cut-off for Node.js 12 is 23rd March. After this time landing SemVer Major changes in v12.x will require no objections from the TSC.

(This issue will be closed after 23rd March - it was opened as an easy way to publicise.)

Trott commented 5 years ago

After this time landing SemVer Major changes in v12.x will require no objections from the TSC.

Feel free to ignore this question as a low-value can of worms that you would like to not open, but if you're into worms like I am:

Should that be the Release WG rather than TSC? Or is Release WG saying "TSC approval is the process for post-cutoff semver majors in this release"? Or something else?

I'm asking because, if I'm not mistaken, TSC delegated release contents to the Release WG in the Release WG charter. Only way TSC can have a say is if they revoke the charter or if the Release WG grants it to them.

mcollina commented 5 years ago

If I recall correctly, lwe have landed semver-major commits with TSC approval in this phase for the last few releases.

cc @jasnell

jasnell commented 5 years ago

@mcollina and @BethGriggs are correct, after the cut off it's always been semver-major only with no objections from TSC. The release wg can certainly weigh in on the discussion, as can anyone else.

@targos ... Where are things at with regard to v8 updates and necessary abi stability patches?

targos commented 5 years ago

@jasnell The V8 version that we would like to have in Node.js 12 is 7.4. PR: https://github.com/nodejs/node/pull/26685 The PR includes what's needed to make it ABI compatible with 7.5 (@addaleax can you confirm?). We have issues building on macOS (only in CI apparently, I think @ryzokuken could build locally) and AIX.

addaleax commented 5 years ago

The PR includes what's needed to make it ABI compatible with 7.5 (@addaleax can you confirm?).

It provides ABI compatible with V8 lkgr at the time of those patches – that goes further than V8 7.5, and we would need to re-float some of the patches after upgrading to V8 7.5 to keep ABI compatibility, with the goal of possibly enabling another V8 bump. If we decide to stick with V8 7.5, we can probably omit a number of patches.

richardlau commented 5 years ago

The plan was to stop at V8 7.6: https://github.com/nodejs/node/issues/25082

Trott commented 5 years ago

@mcollina and @BethGriggs are correct, after the cut off it's always been semver-major only with no objections from TSC. The release wg can certainly weigh in on the discussion, as can anyone else.

Can of worms opened!

This practice, however long-standing, contradicts the Release WG charter which says the Release WG, not the TSC, has final authority over what goes in a release. (The TSC determines what goes in the master branch.)

As it says in the TSC repo material about working groups:

Once formed the work defined in the Working Group charter is the responsibility of the WG rather than the TSC.

So, TSC does not have authority over what ends up in a release. Release WG does. Which is why I asked:

Or is Release WG saying "TSC approval is the process for post-cutoff semver majors in this release"?

That's probably the easiest solution to aligning what's actually happening with the designation of responsibilities in the WG charter.

ofrobots commented 5 years ago

This discussion is infact about what goes into the master branch. Ultimately this flows into the 12 release, hence the confusion in terminology. My take is that tsc decision applies until the release wg actively working on release content.

On Thu, Mar 21, 2019, 4:53 PM Rich Trott notifications@github.com wrote:

@mcollina https://github.com/mcollina and @BethGriggs https://github.com/BethGriggs are correct, after the cut off it's always been semver-major only with no objections from TSC. The release wg can certainly weigh in on the discussion, as can anyone else.

Can of worms opened!

This practice, however long-standing, contradicts the Release WG charter which says the Release WG, not the TSC, has final authority over what goes in a release. (The TSC determines what goes in the master branch.)

As it says in the TSC repo material about working groups https://github.com/nodejs/TSC/blob/a1f4f6fd6fc259996320fee10080d504df82f275/WORKING_GROUPS.md#nodejs-core-working-groups :

Once formed the work defined in the Working Group charter is the responsibility of the WG rather than the TSC.

So, TSC does not have authority over what ends up in a release. Release WG does. Which is why I asked:

Or is Release WG saying "TSC approval is the process for post-cutoff semver majors in this release"?

That's probably the easiest solution to aligning what's actually happening with the designation of responsibilities in the WG charter.

— You are receiving this because you are on a team that was mentioned. Reply to this email directly, view it on GitHub https://github.com/nodejs/node/issues/26844#issuecomment-475445928, or mute the thread https://github.com/notifications/unsubscribe-auth/AAE0qYjNsAmFJxjAqCUaZTdeBoAL8oYWks5vZBuSgaJpZM4cCCAG .

refack commented 5 years ago

We have issues building on macOS (only in CI apparently, I think @ryzokuken could build locally) and AIX.

Seems like it builds with XCode9 on macOS 10.12 https://ci.nodejs.org/job/temp-node-test-commit-osx/1/nodes=osx1012/ (just 1 failing test)

Trott commented 5 years ago

This discussion is in fact about what goes into the master branch.

@ofrobots Maybe I'm misunderstanding you, but it seems to me that it's pretty definitely about what goes in the release branch, not master:

...landing SemVer Major changes in v12.x will require no objections from the TSC.

That says that after the cut-off date, TSC can block semver majors from landing in the release branch, even though they've landed in master. But only the Release WG can decide what does and doesn't land in 12.x. (If they're voluntarily deferring to the TSC, cool. But there's no need for that and it would have to be something the Release WG as a whole decided.)

Trott commented 5 years ago

I'll stop now before I burn bridges that can't be rebuilt. I'm sure at least some people are beyond exasperated with me at this point on this topic.

This all does bring up one important issues for consideration elsewhere: The project as a whole often engages in practices that diverge from documented policies. We should not do that. Policies and practices should be updated to align.

And now I'll do this...

homer

gireeshpunathil commented 5 years ago

I agree with @Trott :

So in this context, it should be release WG's discretion to define the content, between the cutoff date and the actual release - I think this discretion should also be a major role of that working group .

However, if the matter at hand has implications that cannot be evaluated fully within the release WG's capacity, they can always ask for advise from TSC? (but should be fully at their discretion)

richardlau commented 5 years ago

This is the first major release of Node.js post-convergence that isn't being done by James. As such we are making an effort to document the steps so it can be repeated, and this is currently being PR'ed over in https://github.com/nodejs/node/pull/25497 and would probably be a better place for discussing given that in its current state it is proposing:

One month or less before the release date, commits must be cherry-picked in to the two branches. To land SEMVER-MAJOR at this time requires no objections from the TSC.

Trott commented 5 years ago

On a totally different note: OMG, thanks for all the hard Release work! The activities of Release WG (and Build WG for that matter) can't be appreciated enough and it's not my intention to make things more work than they already are. (But since that's the likely short-term effect, it's another reason for me to stop already.)

jasnell commented 5 years ago

Fwiw, the fact that release wg has never raised an issue on this in 8 major releases where we've followed this approach is relevant. I think there's generally consensus that asking the TSC for objections is an ok thing to do. And, to be honest, doing so doesn't violate any existing documented policies given that it's members of the release wg that are asking the TSC for any objections.

ofrobots commented 5 years ago

v12.x has already been created? ( checks date, gasps ).

I've always interpreted things to be that the TSC defines the semver major content for a new major, and that the Release WG takes ownership of the contents from thereon. I'm happy to stand corrected if the documented policies are difference.

vitaly-t commented 5 years ago

Are we going to see V8 7.2 within Node 11 any time soon? Native support for Private Properties is going to change the landscape for the JavaScript developers. Can't wait!!!

richardlau commented 5 years ago

Are we going to see V8 7.2 within Node 11 any time soon? Native support for Private Properties is going to change the landscape for the JavaScript developers. Can't wait!!!

There are no plans to update the version of V8 in Node.js 11. V8 version increments are usually ABI incompatible with previous versions without floating patches on them and we normally only do that for the even numbered releases that will eventually become Long Term Support (LTS) releases. So to get a newer version of V8 than is currently in Node.js 11 you'll need to pick up Node.js 12 when it is released in April.

Re. V8 7.2 we didn't even land it in master (we skipped straight from 7.1 to 7.3, see https://github.com/nodejs/node/pull/24875 and https://github.com/nodejs/node/pull/25852).

BethGriggs commented 5 years ago

@nodejs/tsc - could you review the following Semver Majors and let me know if there are any objections to any of them landing? Please 👍 if you have seen the list and have no objections.

Semver-Major's that have landed since 23rd March:

rvagg commented 5 years ago

I can't give a :+1: to "modules: throw an error for invalid package.json main entries #26823", it looks risky, but I also don't have time right now to dig into details so I can only abstain for now. 👍 to the rest though. Thanks for the cat herding @BethGriggs.

BethGriggs commented 5 years ago

Has someone turned on protection on the v12.x branch?

devsnek commented 5 years ago

https://github.com/nodejs/node/commit/15c0947fee2078bae4af1404e64448bf959a53bb got a lot of tsc signoff but isn't listed here, is it possible for it to be in 12.x?

mhdawson commented 5 years ago

I'm in agreement with @rvagg they all look good to me except #26823 which could use more time in master. Is there any rush/need to get that into 12.x?

BridgeAR commented 5 years ago

There is no rush but I would like to include it in v12. Is there any benefit in having it on master longer? I can't think of any way that this would be beneficial for this case as it's more like a bug fix for quite a rare case (it's not about trying out a new feature or something like that).

targos commented 5 years ago

@BethGriggs

Has someone turned on protection on the v12.x branch?

It is turned on for the wild card v*.x.

BethGriggs commented 5 years ago

Ah! - that makes sense. I was thinking it'd be nice to pull these majors over in the same order in which they landed on master. I can cherry-pick them on top - is that what used to happen @jasnell?

sam-github commented 5 years ago

Maybe there shouldn't be a v12.x until its actually released? just -rc branches cut from v12.x-staging?

jasnell commented 5 years ago

@BethGriggs.. yeah, prior to the actual release, after the semver-major cutoff, I would start cherry-picking into the vN.x-staging branch in the order they landed as much as possible. There were times when the order couldn't be perfectly preserved. Then I would sync the vN.x branch with the vN.x-staging then rebase the vN.0.0-proposal branch on the vN.x. Cut your test builds and release candidates from the vN.0.0-proposal branch as you. When you're ready to cut the release, you simply reverse the flow... merge vN0.0.0-proposal into vN.x, then rebase vN.x-staging on vN.x, and you're good to go. The flow is fairly straightforward.

BethGriggs commented 5 years ago

Another round of majors that have landed on master:

@nodejs/tsc please let me know if you have objections to any of these landing (or add a 👍so we know that you've seen the list). I expect to pull in the final set next week.

mhdawson commented 5 years ago

On #23027 I wonder if we should wait for https://github.com/nodejs/node/pull/27179 which is related and a follow on due to concerns over the smaller size introduced in #23027. @mcollina what do you think?

I'm ok with the rest.

mcollina commented 5 years ago

Maybe it’s better to wait, true.

MylesBorins commented 5 years ago

So qq… if we have semver-major changes landing the we don’t want to have in 12.x should we back them out? Starting off a new release with all sorts of conflicts seems like a pretty bad way to kick off an LTS release

On Apr 11, 2019, at 6:26 PM, Matteo Collina notifications@github.com wrote:

Maybe it’s better to wait, true.

— You are receiving this because you are on a team that was mentioned. Reply to this email directly, view it on GitHub https://github.com/nodejs/node/issues/26844#issuecomment-482349595, or mute the thread https://github.com/notifications/unsubscribe-auth/AAecV138hSMFJZHn4AkVhbGEG9Y3HG7cks5vf7aSgaJpZM4cCCAG.

tniessen commented 5 years ago

Node.js 12 has been released. Closing and unpinning.