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

Proposal: change deprecation policy #13001

Closed
toddself opened this issue May 12, 2017 · 24 comments
Closed

Proposal: change deprecation policy #13001

toddself opened this issue May 12, 2017 · 24 comments
Labels
meta Issues and PRs related to the general management of the project.

Comments

@toddself
Copy link
Contributor

The current node deprecation policy is very difficult for certain sets of node users to find out about breaking changes between LTS-enabled node versions.

With node's tick/tock even/odd release cycle where odd-numbered releases will never enter an LTS mode, it is very possible to miss the deprecation of a much-relied upon feature because there's no business reason for me to test my code against a version of node that I will never release my services on top of.

As with #12976, many people who rely on not having to provide callbacks to asynchronous functions would have been caught blind by migrating from 6 to 8, skipping over the deprecation warning entirely and going from "this works" to "this kills my node process entirely"

Given the speed at which the node community releases major versions (4 was released like barely over a year ago and we're almost to 8!), it would be extremely beneficial to extend the deprecation period to always include an LTS-enabled version of the project. This way us "leapfroggers" don't have to deal with vastly unexpected issues with our code bases and can provide feedback on breaking changes during an appropriate window without the added baggage of trying to get a rollback performed.

@mscdex mscdex added the meta Issues and PRs related to the general management of the project. label May 12, 2017
@ChALkeR
Copy link
Member

ChALkeR commented May 12, 2017

it is very possible to miss the deprecation of a much-relied upon feature because there's no business reason for me to test my code against a version of node that I will never release my services on top of.

I (personally) believe that this is a self-contradictory statement and suggest everyone to note that.


So, do I get it correct that the proposal is to slow down the deprecation process twice and account only LTS releases? There would be significant drawbacks if that would be enforced…

That said, I'm not saying that I am strictly against this proposal now (more like a -0 currently) — I haven't collected and estimate the data on this yet.

/cc @nodejs/ctc, btw, as this proposes a serious process change.

@toddself
Copy link
Contributor Author

I (personally) believe that this is a self-contradictory statement and suggest everyone to note that.

fs.writeFile's callback omission was deprecated in 7. 7 will never enter LTS. I have not run it anywhere (even in CI), nor do I have plans to.

I will switch to Node 8 when it enters LTS. Because I will never run node 7 anywhere I would miss this deprecation entirely. From my perspective it would be working in node 6, but broken in node 8 (and broken in a way that would cause my services to stop running due to uncaught exceptions being thrown).

Deprecations serve as a way to inform your user base a breaking change is coming. If we miss the deprecation because it's being done on a version that we aren't using, what purpose does the deprecation serve? We are unable to properly act upon it since we didn't ever actually get the deprecation.

I would love for deprecations on functionality on at least modules marked stable to go through at least ONE LTS version before actually being deprecated.

eg: fs.writeFile is marked as deprecated in 7 AND 8, and then removed in 9. Or it's deprecated in 8 and removed in 9.

@jasnell
Copy link
Member

jasnell commented May 12, 2017

hmmm.. a quick clarifying question: would the intention of this be that some bit of Deprecated code could not be EOL'd until after the LTS cycle completes?

For instance, let's say we deprecate some function today in master. That deprecation will occur 8.0.0. Let's say we get another semver-major PR in june that EOL's that code and removes it entirely. That EOL would not go into a release until 9.0.0 next October.

With the proposed change: Is that OK since the deprecation would include an LTS release branch (that is, the API is guaranteed to exist through the lifetime of 8.x but disappear from 9.0.0 onwards). Or... can the code not be EOL'd at all until 8.x LTS expires? (Which means not landing the EOL until Node 14!!)

@mscdex
Copy link
Contributor

mscdex commented May 12, 2017

I'm curious as to why you would explicitly not test non-LTS releases (especially in CI)?

@toddself
Copy link
Contributor Author

I'm curious as to why you would explicitly not test non-LTS releases (especially in CI)?

A lot of reasons? I don't have the time to setup CI to run on multiple node versions, our CI success gates whether or not something can be merged, so we'd then have to fix things that aren't broken, or get fixes from upstream providers (or upstream of upstream, etc), the list sort of goes on from there, but I think those two issues should suffice enough to satisfy that curiousity.

For instance, let's say we deprecate some function today in master. That deprecation will occur 8.0.0. Let's say we get another semver-major PR in june that EOL's that code and removes it entirely. That EOL would not go into a release until 9.0.0 next October.

That seems to be the current policy right? Node gets semver-major every 6 months. 9 comes out in Oct 2017. If 8 wasn't late due to turbofan (again a great reason to delay it!), that means something you want to deprecate today would not be able to be removed until Node 11 in Oct 2018 (since it would be deprecated in 9 and 10). This gives the ecosystem adequate time to catch up to larger scale changes.

Again though, this should be only used when making a semver major change to an API marked as stable. You wanna deprecate something with N-API, follow the current deprecation rules since that module is marked as unstable.

@Trott
Copy link
Member

Trott commented May 13, 2017

I thought runtime deprecation warnings are supposed to be in place for two major releases specifically to guarantee that they end up in an LTS release before anything gets removed. @jasnell Isn't that the intention of the deprecation policy you drafted at https://github.com/nodejs/node/blob/98609fc1c4635f02f8f6ef9dd079207a1204b9a1/COLLABORATOR_GUIDE.md#deprecations?

@toddself Am I misunderstanding you? Or is that basically what you're asking for?

If I'm right and that's part of our policy, then the problem may be that we haven't been following our policy. If that's the case, there are a few possible causes. One is that the policy seems a bit confusingly worded. End-of-Life Deprecation is basically the same thing as Runtime Deprecation. So something is in Runtime Deprecation for a major release. Then it's in End-of-Life Deprecation for a major release, which is the same thing--just a name change. Then, at some point in the future, it may get removed. At least that's how I'm reading it. Am I wrong? If I'm not wrong (and maybe even if I am), then a simplification/clarification of the existing policy text might be useful.

@jasnell
Copy link
Member

jasnell commented May 13, 2017

The End-of-Life phase is currently intentionally non-committal as to when the code can actually be removed (or the behavior changed). Within the current policy, the idea is that End-of-Life code can be changed or removed at any time following the move to End-of-Life. Moving code to End-of-Life is a semver-major action so it is only supposed to happen with a major release.

(In other words, code that is in End-of-Life status has essential the same semver status as code in Experimental status... breaking changes can occur at any time within a major.. so long as the move to End-of-Life itself is consider a semver-major change)

@Trott
Copy link
Member

Trott commented May 13, 2017

(In other words, code that is in End-of-Life status has essential the same semver status as code in Experimental status... breaking changes can occur at any time within a major.. so long as the move to End-of-Life itself is consider a semver-major change)

@jasnell With the understanding that you are free to change your mind after more thinking about it, what's your immediate reaction to "We ought to generally require runtime deprecation for two major releases?" Yes, I like it? No, I hate it? Not sure, gotta think about it?

(The "generally" in there is to acknowledge that there needs to be an exception process of some kind for things like what happened with the legacy debug API.)

@mscdex
Copy link
Contributor

mscdex commented May 13, 2017

"We ought to generally require runtime deprecation for two major releases?"

That wouldn't necessarily need to be the case if the main focus is on LTS. For example, if a runtime deprecation first happens on v8.x, then why would we necessarily have to extend that through v9.x as well?

@jasnell
Copy link
Member

jasnell commented May 13, 2017

@Trott ... i think there are far too many variables to consider to establish two majors as a minimum. For instance, we may be deprecating some bit of code because some mechanism in V8 is expected to disappear entirely soon after cutting the next major. Or, we may be deprecating because of a security or usability issue. Or we may just be deprecating because we just have a much better way of doing things in a different way. Pushing the runtime deprecation out to a minimum of two majors would not make any of these any easier for the ecosystem to cope with.

What I would like to see is more emphasis on communicating upcoming deprecation policy transitions. Obviously communicating more effectively is not useful if folks aren't listening or paying attention, but there really is only so much that we can do in that regard.

The short version is: I'm -0 on extending out the runtime deprecation cycle because I don't believe it would actually solve the issue.

@mscdex
Copy link
Contributor

mscdex commented May 13, 2017

I agree. Personally I still don't understand the argument that adding another node version in CI is troublesome, especially if it allows you to catch runtime deprecations early (in case the documentation deprecations were not read), which would solve the original issue. If the issue is there is a bug in a non-LTS node version, then an issue/PR should be created so the problem can be fixed, just like if a bug were to happen in an LTS branch (especially because there is a good chance the bug may be present in the LTS branch as well).

@fl0w
Copy link

fl0w commented May 13, 2017

Wouldn't it be easier to backport deprecations to LTS as semver-patches instead of extending the lifecycle of deprecated features (if running unstable on CI isn't an acceptable practice)?

@jasnell
Copy link
Member

jasnell commented May 13, 2017

runtime deprecations cannot be semver-patch because they have side effects that can break running code (stderr output and new throws when --throw-deprecation is used).

@toddself
Copy link
Contributor Author

toddself commented May 13, 2017 via email

@toddself
Copy link
Contributor Author

@Trott that is even more than I'm asking for (but it would be great). I'm asking that all deprecations land in an LTS release before being removed.

I don't even mind if that LTS version gives me a console output when I use something deprecated, I would just love to not have to track the unstable and experimental versions of node.

Also @mscdex, why would I want to test in CI on a version of node that is unstable, even if I had the time. The odd numbered releases aren't supposed to be "stable" even if they're released according to the release policy.

@mscdex
Copy link
Contributor

mscdex commented May 13, 2017

@toddself I wouldn't consider non-LTS unstable. AFAIK the only real difference between LTS and non-LTS is how long the branch is supported. Semver and other rules still apply for any branch. From what I've seen, many users are willing to use non-LTS branches if it means they get some new fancy feature (e.g. async/await in v7.x). Because of that, testing against non-LTS branches is beneficial IMHO at least for that reason.

As far as supporting non-LTS in a CI environment goes, with Travis for example it's as simple as adding a single line to the config file to have it run against whatever node version I want. Again, to me that is beneficial because it allows me to find out about semver-major changes or deprecations that I may have missed. There really is no significant time investment needed for this IMHO.

@jasnell
Copy link
Member

jasnell commented May 13, 2017

@toddself ... I don't want to leave the wrong impression that I'm not sensitive to the issue you're dealing with. I've had a number of folks ask about the stability of the odd numbered majors and whether they would be considered to be "experimental" or not. The short answer is that they're not experimental and are just as stable as the even numbered releases, they just aren't supported for as long. The other key difference is that once an even numbered release does go into LTS, we slow down the rate at which semver-minor commits land. There are quite a few users who use odd numbered releases in production, but you are absolutely correct that many users do not transition until the even numbered majors go into LTS.

I would be amenable to the suggested policy that runtime deprecations have to sit through at least one even numbered major because going to End-of-Life, albeit with allowance for overriding the policy on a case by case basis. The short version of this is that moving to End-of-Life would only happen in odd numbered majors.

@MylesBorins
Copy link
Contributor

MylesBorins commented May 15, 2017

@jasnell that isn't exactly accurate.

All current releases take any commit that isn't semver major on a weekly basis. LTS requires them to bake before landing. That by itself is a fairly major difference in stability.

I think that it is reasonable that we have the default of two releases before removing things... I also think the language we have is very confusing.

The short version of this is that moving to End-of-Life would only happen in odd numbered majors.

I don't think this has to happen if we do two releases... we just need to make sure that there is at least ONE lts release that has the warning... the warning could come in even and the feature removed in even as well

@sam-github
Copy link
Contributor

Fwiw, what I see with the enterprise customers I talk to is they tend to take periodic large changes over continuous small. So, they will never run non-LTS, and probably only use every-other LTS (we've a number updating from 0.10 to 4.x, and others on 4.x expecting to go to 8.x - or even 9 if they can get away with it). They expect majors to involve incompatible change, they often expect the majors to be harder to update to than they actually are.

IMO, the two-major deprecation rule allows people who want sneak previews, particularly npm package authors who want their packages to be ready for the next major the instant it drops, to run tests on ci (trivial with Travis, for example) and ensure their tests pass with no warnings.

For other users, if they only update and test on LTS after it is released, that's fine, too. Its a major release, somewhat backwards incompatible by definition, and once they've read the release notes and got CI working, they should understand the differences.

@mhdawson
Copy link
Member

I'm in agreement with @MylesBorins in that we (as in IBM) only recommend use of LTS releases in production which means people who follow that advice may not see the deprecation warnings. I also see @sam-github's point that even if it was 2 LTS releases that might not help all customers and that as a major release you still need to plan on testing/validation due to breaking changes (which may or many not have had deprecation warnings). I think it comes down to benefits/cost. Unless we need to deprecate faster for some reason then ensuring the deprecation is in at least one LTS seems to be reasonable.

@sam-github
Copy link
Contributor

When we deprecate, its because we want something gone, so we do want to float deprecations for as short a time as is reasonable. I'm not sure exactly where the balance is, but until now, I hadn't heard any complaints about the two majors. I think we already do it at a pretty measured pace, particularly considering semver would suggest every major can be breaking, and doesn't require deprecation periods. Its node that considers the mere introduction of a deprecation warning to be a breaking change, so deprecations are deferred to the next major, and removal to the one after that. This proposal would make it 2 LTS, which is almost two years!

I'm not terribly opposed to slowing it down, but would like more people to step up saying that the 2 majors wasn't enough.

And if we are slowing it down, a criteria that would be useful is to not remove something until a replacement for it exists in all maintained LTS releases. This criteria could be achieved by waiting until older LTS that don't have the replacement go out of support, or by backporting to older LTS. The goal would be that code can be written at all times that works across all LTS (albeit with deprecation warnings).

4 was released like barely over a year ago

4.0.0 was released September 8, 2015

@jasnell
Copy link
Member

jasnell commented May 17, 2017

I'd rather make the policy simply: transition to End-of-Life can only happen in non-LTS majors (so odd majors only)... with exception given on a case by case basis as needed.

@Trott
Copy link
Member

Trott commented Aug 13, 2017

Should this remain open? Does current deprecation policy reflect community needs?

@BridgeAR
Copy link
Member

BridgeAR commented Feb 9, 2018

I am closing this because there has not been any activity anymore for a long time here. If someone disagrees, please leave a comment or reopen.

@BridgeAR BridgeAR closed this as completed Feb 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta Issues and PRs related to the general management of the project.
Projects
None yet
Development

No branches or pull requests

10 participants