Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Tracking] Additional semver minor to 8.x during maintenance #405

Closed
MylesBorins opened this issue Jan 3, 2019 · 23 comments
Closed

[Tracking] Additional semver minor to 8.x during maintenance #405

MylesBorins opened this issue Jan 3, 2019 · 23 comments

Comments

@MylesBorins
Copy link
Contributor

MylesBorins commented Jan 3, 2019

There are a couple different features that we might want to ship before 8.x goes EOL

As such I think it does make sense for us to do one more Semver-Minor relase of the 8.x release line.

Thoughts?

@mhdawson
Copy link
Member

mhdawson commented Jan 4, 2019

@MylesBorins I assume you mean 8.x in your original post as opposed to 10.x.

@MylesBorins
Copy link
Contributor Author

@mhdawson fixed, thanks

@richardlau
Copy link
Member

richardlau commented Jan 15, 2019

I'd like to get nodejs/node#25447 in if we do another 8.x release as it fixes a regression introduced in 8.11.2. Let me know if it needs a manual backport.

@MylesBorins
Copy link
Contributor Author

@richardlau can you please backport?

@richardlau
Copy link
Member

@richardlau can you please backport?

Done! nodejs/node#25521

@mhdawson
Copy link
Member

@MylesBorins what timing are you thinking for the release. Just want to make sure we sync with @gabrielschulhof to see if there is any chance it can also include the PR to make tsfn non-experiemental as wel as opposed that coming later.

@MylesBorins
Copy link
Contributor Author

I'm not in a rush tbqh, maybe end of feb / march

@MylesBorins
Copy link
Contributor Author

Removing label as we had consensus to do this release.

We should keep this open for tracking

@MylesBorins MylesBorins changed the title Additional semver minor to 8.x during maintenance [Tracking] Additional semver minor to 8.x during maintenance Jan 24, 2019
@ljharb
Copy link
Member

ljharb commented Feb 6, 2019

It would be really nice to backport the instanceof Array fixes from https://github.com/nodejs/node/pull/20250/files#diff-286202fdbdd74ede6f5f5334b6176b5cL320 into 8 (and even 6). It breaks jest, and cross-realm use cases, and I can't conceive of how the fix would cause any trouble.

@MylesBorins
Copy link
Contributor Author

@nodejs/collaborators quick ping. Is there anything specific you would like to see on 8.x?

@BridgeAR
Copy link
Member

I would like to include nodejs/node#26599.

@lpinca
Copy link
Member

lpinca commented Mar 19, 2019

nodejs/node#25938

@Trott
Copy link
Member

Trott commented Mar 19, 2019

From https://github.com/nodejs/Release#release-plan:

Once a major version enters LTS coverage, new features (semver-minor) may only be landed with consent of the Release working group.

And...

Changes in an LTS-covered major version are limited to:

  1. Bug fixes;
  2. Security updates;
  3. Non-semver-major npm updates;
  4. Relevant documentation updates;
  5. Certain performance improvements where the risk of breaking existing
    applications is minimal;
  6. Changes that introduce large amount of code churn where the risk of breaking
    existing applications is low and where the change in question may
    significantly ease the ability to backport future changes due to the
    reduction in diff noise.

This would seem to preclude things like namespaces or new CLI flags.

If the material quoted above is accurate, then I assume Release WG will reject adding --unhandled-rejections to 8.x (or for that matter 10.x).

If the material quoted above is not accurate, then can we please update it? (Pro-tip: When feasible, removing wrong text is easier than trying to correct text that isn't necessary.)

@Trott
Copy link
Member

Trott commented Mar 19, 2019

(For the record, I'm fine one way or the other with --unhandled-rejections in LTS. I just want the quoted document to be correct.)

@MylesBorins
Copy link
Contributor Author

@Trott we likely need to update the language in the second area. We've mostly run with "we land whatever we agree as a group to land". That list is more for pointing people at when we agree not to land something...

We have landed new flags in prior LTS minors afaik

@gabrielschulhof
Copy link

nodejs/node#25648 is semver-minor and needed for N-API and nodejs/node#26060 is the backport of a N-API bug fix.

Trott added a commit to Trott/LTS that referenced this issue Mar 20, 2019
The README had a list that was supposedly the only types of changes that
land in LTS branches. However, other types of changes have landed there
routinely. Rather than try to codify, removal is a tacit acceptance that
we land whatever Release WG deems appropriate.

Refs: nodejs#405 (comment)
Refs: nodejs#405 (comment)
@Trott
Copy link
Member

Trott commented Mar 20, 2019

@MylesBorins This paragraph probably also needs to go, since here we are talking about landing all sorts of non-critical stuff in a maintenance release.

Once a release moves into Maintenance mode, only critical bugs,
critical security fixes, documentation updates, and updates to ensure
consistency and usability of the N-API across LTS releases will be permitted.
Unless a change is urgent it will be planned into a release once per
quarter. Such releases will only be made when necessary.

@MylesBorins
Copy link
Contributor Author

@Trott we need to discuss this as a team and figure out what we want to do here.

That paragraph is important and has generally been a guiding start... keep in mind that 8.x had a shortened maintenance release cycle... we have not decided on exactly which PRs we are going to land. It initially started due to the need for specific n-api stuff... and we have documentation somewhere (unsure where) about n-api updates themselves being different.

We might want to scale back how ambitious we are going to be about this maintenance LTS release, but we still haven't had a WG meeting to review and discuss. Alternatively it might be time to sit down and rethink how we are doing LTS based on past experience and the new folks working on it.

@mcollina
Copy link
Member

I think @BethGriggs included nodejs/node#25351 already.

@mhdawson
Copy link
Member

mhdawson commented Mar 20, 2019

This

Once a release moves into Maintenance mode, only critical bugs, critical security fixes, documentation updates, and updates to ensure consistency and usability of the N-API across LTS releases will be permitted. Unless a change is urgent it will be planned into a release once per quarter. Such releases will only be made when necessary.

Is the text related to N-API and is 2 paragraphs below the text @Trott referenced in the README.md

@MylesBorins
Copy link
Contributor Author

PR is open

nodejs/node#26933

Anything missing?

@mhdawson
Copy link
Member

mhdawson commented Apr 2, 2019

@gabrielschulhof can you take a quick look to verify the list as well?

@BethGriggs
Copy link
Member

v8.16.0 was released nodejs/node#26933

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants