Node: API stability level «Locked» is inaccurate
Atm, these modules are listed in the documentation as being Locked: assert, modules, timers.
_Update: assert was successfully unlocked in #11304. :tada:_
_Update: timers was successfully unlocked in #11580. :tada:_
Locked is defined as:
Stability: 3 - Locked
Only fixes related to security, performance, or bug fixes will be accepted.
Please do not suggest API changes in this area; they will be refused.
Still we have those changes recently landed (~ 1 year):
assert: #3374, #639, #668, #308 (ok, that one is present since 1.0.0), borderline features/bug fixes: #2407, #4166, #3276, #636,modules: #5950, #5689, #1812, #1363, #1185, #1162,timers: #4362, #3374, https://github.com/nodejs/node-v0.x-archive/pull/8884.~~
And more proposed: #10282, #3384, #6165, #4550 (ok, the last two are not documented).
How Locked is defined does not fall in line with what's actually going on there. It looks more like stability level Stable should be used instead:
Stability: 2 - Stable
The API has proven satisfactory. Compatibility with the npm ecosystem
is a high priority, and will not be broken unless absolutely necessary.
Perhaps we should remove Locked stability level whatsoever?
/cc @nodejs/ctc
ChALkeR
All 22 comments
This has been discussed before. As much as I don't like it, we kinda need to strongly signal we don't take much in terms of features so some modules.
Locked basically means: avoid PRing additions or breaking changes. (In a strong way.)
It's mostly to keep people from willy-nilly proposing _anything_.
Fishrock123
on 2 May 2016
That being said, timers is the least locked in the bunch, but we won't normally take features for it.
Fishrock123
on 2 May 2016
@Fishrock123 Well, it doesn't work. It should be either followed, reworded in the docs, or removed completely.
ChALkeR
on 2 May 2016
Only fixes related to security, performance, or bug fixes will be regularly accepted.
Please do not suggest API changes in this area; they will probably be refused.
@ChALkeR that wording work for you?
Fishrock123
on 3 May 2016
Only modifications related to security, performance, and bug fixes will be accepted.
Requests for new features or API changes will not be considered.
jasnell
on 3 May 2016
@jasnell For example, #639 was clearly a feature, and what's going on with module are API changes.
ChALkeR
on 3 May 2016
Rewording in order to reflect more accurately the current flow seems like an improvement for me. @nodejs/documentation
a0viedo
on 3 May 2016
I think making clear the actual purpose of "Locked" is a good idea. Is it as @Fishrock123 has proposed?:
It's mostly to keep people from willy-nilly proposing anything.
Has "Locked" had that affect vs. modules marked as "Stable"? Has anyone compiled any data on it?
It is a good idea to say that there can never be any improvements (additional features) added to a module? Is that better than saying such suggestions are strongly discouraged and likely to be ignored? Understanding the purpose of marking a module as a "Locked" (especially vs. "Stable") is important with respect to determining how/when it should be used and what documentation should go along with that label. What other systems go out of their way to make that differentiation -- and what has been the outcome?
dlongley
on 3 May 2016
@Fishrock123, @jasnell The issue here is that the description of the «Locked» API stability level is actually misleading, given what is actually going on.
New features (new methods, documented), new throws, behaviour changes — all of that has been recently landed to the modules that are «Locked» per the documentation.
It looks more like a form of «Very Stable» to me, but I have no idea yet of how should that be documented.
ChALkeR
on 3 May 2016
Has "Locked" had that affect vs. modules marked as "Stable"?
Yes. We semi-regularly reject feature requests and/or major changes in those areas.
Fishrock123
on 3 May 2016
@Fishrock123 Even if it works that way, the feature requests and/or major changes that slip through are breaking user expectations there.
ChALkeR
on 4 May 2016
Should just rename it to LockedUnlessOtherwiseDeemedNecessary
trevnorris
on 7 May 2016
Considering this has come up a few times most recently in https://github.com/nodejs/node/pull/3384 do you think it makes sense to bring this up with the ctc in the next meeting?
MylesBorins
on 18 May 2016
I'd say we should rephrase Locked slightly, if anything. Useful features like https://github.com/nodejs/node/commit/6fc5e953547bfbdf97bc380d4bfc471e3378fad9 shouldn't be blocked through a policy like this.
silverwind
on 18 May 2016
To be honest I would personally like to see us revisit these levels entirely now that we have a solid LTS process in place. Technically, every API in an LTS release is Locked and the labels make very little sense there. In master, we should have the freedom to make the changes we collective feel are necessary to make while _still_ erring on the side of being overly conservative. I'm not saying that we would get rid of the labels entirely, just that we should take a step back and reevaluate them.
jasnell
on 18 May 2016
@jasnell On the other hand: especially now that for many people (looking at us for example) there's at least one year where we don't see anything happening on master, having strict stability of certain APIs across LTS versions allows us to trust that we don't have to "rewrite" our apps for the next LTS of node. So far almost every node upgrade across major versions (0.8/0.10/various io.js/4) has been very smooth and only required some limited, straight-forward work ("method X was removed, it's now Y, do this to support both"). Allowing subtle changes to the module system between LTS versions will make upgrades a lot scarier.
jkrems
on 26 May 2016
jasnell
on 6 Aug 2016
@jasnell That doesn't say anything about the API stability levels or clarify what changes could be landed there, though.
ChALkeR
on 6 Aug 2016
I know, was just linking the issues. There's more that works need to be done
jasnell
on 6 Aug 2016
Status update: https://github.com/nodejs/node/pull/11304 landed, assert is not Locked anymore.
Only module and timers are left.
ChALkeR
on 15 Feb 2017
Ah, for issue linking purposes: https://github.com/nodejs/node/issues/11200 has the new discussion about changing the Locked description and/or unlocking the Locked API.
ChALkeR
on 15 Feb 2017
Fixed in 51cea054a276c6425e26b25219e67b37207dee3f
Trott
on 6 Mar 2017
Related issues
filipesilvaa
·
3Comments
cong88
·
3Comments
sandeepks1
·
3Comments
dfahlander
·
3Comments
danialkhansari
·
3Comments
Most helpful comment
To be honest I would personally like to see us revisit these levels entirely now that we have a solid LTS process in place. Technically, every API in an LTS release is
Lockedand the labels make very little sense there. Inmaster, we should have the freedom to make the changes we collective feel are necessary to make while _still_ erring on the side of being overly conservative. I'm not saying that we would get rid of the labels entirely, just that we should take a step back and reevaluate them.