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

Remove stability designations in core JS API #930

Closed
mikeal opened this issue Feb 23, 2015 · 18 comments
Closed

Remove stability designations in core JS API #930

mikeal opened this issue Feb 23, 2015 · 18 comments
Labels
doc Issues and PRs related to the documentations.

Comments

@mikeal
Copy link
Contributor

mikeal commented Feb 23, 2015

For several years we've been marking the core API with a sliding scale of "stability."

It seemed like a good idea at the time but is at best, a poorly messaged note about how much work is happening on an API, and at worst an outright lie.

Even if a module has the lowest level of stability attached we may, and actually have been, guaranteeing backwards compatibility if enough of the ecosystem depends on it.

Streams is laughably marked "Unstable" at the moment. While it is true that streams have been changing a huge amount of effort in each change has gone in to guaranteeing backwards compatibility. That kind of compatibility is not what anyone would expect from an API being messaged as "Unstable."

@SomeKittens
Copy link

👍 thanks for this!

@Fishrock123
Copy link
Contributor

Related to #633 'docs: lower the maximum stability level to "stable"'

@rvagg
Copy link
Member

rvagg commented Feb 23, 2015

and discussion in the TC meeting after that

@mikeal
Copy link
Contributor Author

mikeal commented Feb 23, 2015

To follow up, because I know it's going to happen, we could argue for days about a better way to message the likelihood that an API might change but I don't think it's actually useful. This particular form of messaging, a designation per module, has outlived its usefulness and is now actually harmful.

Our ability to add an API, change an API, etc, has a lot to do with how many people depend on it and very little to do with whatever we decided to mark the API as in the docs. We have a system for considering and messaging those changes (TC process and semver) and that should be enough.

If we want to add an API and reserve the right to remove or change it we should not document it in the official documentation.

@Fishrock123
Copy link
Contributor

If we want to add an API and reserve the right to remove or change it we should not document it in the official documentation.

+1

@Fishrock123
Copy link
Contributor

I do think that if we remove them, we need to insert a temporary message on why. It'd probably save us some issues hassle. :)

@benjamingr
Copy link
Member

I'm in favour of this. People have and will always use "less than stable" APIs. Making their stability is pointless.

How are we going to handle things like domains though? Deprecation warnings?

@mikeal
Copy link
Contributor Author

mikeal commented Feb 23, 2015

I do think that if we remove them, we need to insert a temporary message on why. It'd probably save us some issues hassle. :)

That is all on a per-API basis. Not all API is equal, if we're confident nobody is actually using something we will act differently than an API we think even a few people might be dependent on.

@Fishrock123
Copy link
Contributor

How are we going to handle things like domains though? Deprecation warnings?

@benjamingr Domains is already marked as deprecated as of #141, but not yet in code.

That is all on a per-API basis. Not all API is equal, if we're confident nobody is actually using something we will act differently than an API we think even a few people might be dependent on.

@mikeal I think it would be worth just having a standard "what happened to the stability index" link for a little bit.

@mikeal
Copy link
Contributor Author

mikeal commented Feb 23, 2015

Domains is already marked as deprecated

All that we mean by that is "please don't use this." No decision has been made about removing it or how that would even work. It's highly depended on so I'm skeptical it will ever be entirely gone.

@brendanashworth
Copy link
Contributor

👍 so very much. Most of these modules have been around forever anyways.

@chrisdickinson
Copy link
Contributor

If we want to add an API and reserve the right to remove or change it we should not document it in the official documentation.

That sets up a pretty bad incentive for new APIs; and from past experience even the APIs we don't publicly document end up getting used (readline.codePointAt, stream._readableState, etc).

Perhaps it'd be useful to simplify the index to:

  1. Experimental / Flagged. Breaking changes will happen. Usually/always gated by feature flags.
  2. Stable. Breaking changes are heavily discouraged. In use in production.
  3. Locked. Breaking changes are disallowed unless introduced by a security fix.

Or to start including something like a per-{method,topic,class} "introduced in X.Y.Z" / "last updated in X.Y.Z" informational link.

@mikeal
Copy link
Contributor Author

mikeal commented Feb 24, 2015

That sets up a pretty bad incentive for new APIs; and from past experience even the APIs we don't publicly document end up getting used (readline.codePointAt, stream._readableState, etc).

Ya, if something ends up getting used we end up having to support it, that's the whole reason I want to get rid of these designations. Not documenting it is really just a way of trying to hide it and hoping it doesn't get used which is not ideal but can, in some cases, be effective in the short term.

That said, there's not a lot we can do to keep people from using APIs once they are available publicly. Marking them as "Experimental" hasn't worked in the past and I don't see any reason why we would think it's going to work now.

That's why it's still best to build things as independent modules. The way readable-stream is being built is great and we should continue to take that approach whenever possible.

I don't think "simplifying" the designations helps. The reality is that we support an API to the degree that people depend on it and that can't be messaged with any stability label.

@chrisdickinson
Copy link
Contributor

That's why it's still best to build things as independent modules. The way readable-stream is being built is great and we should continue to take that approach whenever possible.

It's not built as an independent module yet – it consumes io.js, not the other way around. There are roadblocks to overcome before the approach of spinning modules out of core is validated. That's somewhat off topic, though.

I don't think "simplifying" the designations helps. The reality is that we support an API to the degree that people depend on it and that can't be messaged with any stability label.

We should message our level of commitment to an API through the docs. If applying the index to entire modules isn't fine grained enough, that's something we can change. If there are too many levels of the index to be useful, we can refine. It's useful to be able to say "the module system is hot lava, do not touch it," both to people looking to make changes and to reviewers of those changes. It's the strongest guarantee we can make. It's also useful to be able to say "this feature is experimental. use it behind a feature flag. it may go away or change" since that gives us a good venue to start baking in NG features. I'm not saying, necessarily, that any of our existing features labeled as "experimental" really are experimental anymore (due, I think, to the the substantial time between 0.10 and the latest release), so we should change that. "Experimental" gives us a way to message things like "promisifying the fs api," and get that rolled out to people who want to play with it ahead of time and shake out the bugs, while still letting us document it.

The stability index has problems as it stands, but I think we {sh,c}ould make it more valuable for our community by simplifying the levels and more thoroughly messaging about when APIs have last changed at a finer grained level, rather than throwing it out entirely.

@mikeal
Copy link
Contributor Author

mikeal commented Feb 24, 2015

We should message our level of commitment to an API through the docs.

Our level of commitment is proportional to how much the API is depended on. If we have a way to message that I'd love to do it but any statement about what we think our level of support is whenever the last time someone decided to edit the docs was, isn't that.

What does a label like "Experimental" have to do with the amount of dependence the ecosystem has on an API? How does this stay up to date? Again, streams are marked as "unstable" so we already know that we're bad at this.

It's not enough to say "this is a better way to message this" because we don't have a process by which we re-consider these labels regularly and re-message them as their usage increases or decreases. Any process that does this would fall victim to endless bike shedding every time we try to change the labels.

Maybe we should find a way to markup the API docs with the following automated information:

  • The number of NPM modules that directly and indirectly rely on an API.
  • The percentage of NPM that uses an API.

To be clear though, I think we should start by just removing these labels from the docs as all of them are inaccurate and harmful as-is. After that we can start postulating on a better way of messaging the status of each API.

@chrisdickinson
Copy link
Contributor

I think the best course of action here is for me to write up a PR where we can continue discussion :)

@chrisdickinson
Copy link
Contributor

Opened #943.

@Fishrock123 Fishrock123 added the doc Issues and PRs related to the documentations. label Feb 26, 2015
chrisdickinson added a commit that referenced this issue Feb 27, 2015
This simplifies the stability index to 4 levels:

0 - deprecated
1 - experimental / feature-flagged
2 - stable
3 - locked

Domains has been downgraded to deprecated, assert has been
downgraded to stable. Timers and Module remain locked. All
other APIs are now stable.

PR-URL: #943
Fixes: #930
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: Vladimir Kurchatkin <[email protected]>
@chrisdickinson
Copy link
Contributor

cf0306c should address this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations.
Projects
None yet
Development

No branches or pull requests

7 participants