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

doc: remove incorrect information from README #426

Closed
wants to merge 2 commits into from

Conversation

Trott
Copy link
Member

@Trott Trott commented 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: #405 (comment)
Refs: #405 (comment)

@MylesBorins @devsnek

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)
Copy link
Contributor

@MylesBorins MylesBorins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't yet have consensus that we are adopting this as a long term change... the 8.x maintenance minor release coming out may be a one off.

I realize that isn't ideal, but I am not in favor of us just removing these constraints as a general guidance for maintenance LTS

README.md Outdated
***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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This paragraph needs to stay -- It basically defines what being in Maintenance mode is. We have been following this for Node.js 6.x.

Node.js 8.x is an oddity in that according to the original schedule it shouldn't be in Maintenance until April but it was brought forward to January at the end of last year. So far nothing in #405 has actually landed in an 8.x release and the things being discussed there are not guaranteed to land either (in fact this very paragraph, unless changed, should be the guidance as to what is accepted).

@Trott
Copy link
Member Author

Trott commented Mar 20, 2019

@richardlau I've restored that paragraph. PTAL.

@Trott
Copy link
Member Author

Trott commented Mar 20, 2019

I am not in favor of us just removing these constraints as a general guidance for maintenance LTS

That's kinda the problem, though. They are presented as constraints but treated as general guidance.

@richardlau richardlau dismissed their stale review March 20, 2019 05:53

Maintenance paragraph restored

@mhdawson
Copy link
Member

mhdawson commented Mar 20, 2019

How about just changing

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

to

Changes in an LTS-covered major version are limited to the following unless reviewed and approved by the Release Working Group members:

@Trott
Copy link
Member Author

Trott commented Mar 20, 2019

Changes in an LTS-covered major version are limited to the following unless reviewed and approved by the Release Working Group members:

Doesn't Active LTS get all semver-patch and semver-minor changes if they cherry-pick cleanly?

@MylesBorins
Copy link
Contributor

MylesBorins commented Mar 21, 2019 via email

@Trott
Copy link
Member Author

Trott commented Mar 21, 2019

Nope. Every commit goes through a separate review with different
expectation then current

I'm not sure what "separate review with different expectation then current" means, but I can also tell this is going nowhere so I'm going to close this.

@Trott Trott closed this Mar 21, 2019
@Trott
Copy link
Member Author

Trott commented Mar 21, 2019

but I can also tell this is going nowhere so I'm going to close this.

Thanks to anonymous friend-who-has-my-back, I'm back here to say: I've been tipped off that the text above sounds snippy and annoyed. And sure enough, it does. Sorry. Didn't mean it that way. Meant it more like this:

I was offering this as a quick remedy but it looks like this isn't going to fly so someone else will have to do a real fix on the doc. Oh, well. I'll close this.

Didn't want to be a jerk to the Release Team, whose heavy workload is inadequately appreciated!

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

Successfully merging this pull request may close these issues.

4 participants