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

sys: Remove after 3 years of deprecation #182

Closed
wants to merge 1 commit into from
Closed

sys: Remove after 3 years of deprecation #182

wants to merge 1 commit into from

Conversation

geek
Copy link
Member

@geek geek commented Dec 19, 2014

Related PR on node.js: nodejs/node-v0.x-archive#8880

Larger issue that I want to see addressed: A formal deprecation policy.

My proposal is that any deprecated code get deleted/obsoleted in the next major version. If this is the case, we can delete a lot of other deprecated code that's rotting away :)

@bnoordhuis
Copy link
Member

Everyone: thoughts on this? Anecdotal: I still see code from time to time that uses the sys module and that would break without prior warning. That makes outright removal unacceptable, IMO.

The sys module was changed to throw an exception in v0.7 and v0.8 but that resulted in nodejs/node-v0.x-archive#3577 and a subsequent revert in v0.8.1.

I'm going to bring this up at the next TC meeting. If we are going to go ahead, then the module should log a warning first for some time, to give people a chance to upgrade their code and dependencies.

@rlidwka
Copy link
Contributor

rlidwka commented Dec 22, 2014

Everyone: thoughts on this?

Burn it with fire:

  1. Any module loader/package manager (browserify, npm, nexe, etc.) that maintains a list of node.js modules will have keep in mind that sys and util are the same.
  2. Any user who find those "sys" references in the old code will have to learn what "sys" is.

I have concerns about people screaming "hey, io.js is not compatible with node.js", but other than that, it'd be nice to finally deprecate sys.

@mikeal
Copy link
Contributor

mikeal commented Dec 22, 2014

What has changed from the last time we tried to deprecate it?

Also, what is the cost of having the link to sys?

@geek
Copy link
Member Author

geek commented Dec 23, 2014

I really want you all to formalize a set of deprecation guidelines you will try to follow.

@bnoordhuis
Copy link
Member

From today's TC meeting: flat out removing is not the way to go but making require('sys') print a deprecation message is acceptable.

@geek If you make the requested changes, I'll land this. Grep lib/ for util.deprecate for examples.

@geek
Copy link
Member Author

geek commented Jan 7, 2015

@bnoordhuis and @rvagg at what point after a function/module is hard deprecated will it be deleted?

@bnoordhuis
Copy link
Member

@geek Probably never. It's not broken and it's not a maintenance burden. Even a deprecation message is somewhat debatable but deprecation messages can be disabled if you really must; at least it shoos people away from using 'sys' in new code.

@vkurchatkin
Copy link
Contributor

@bnoordhuis is this specific to sys or you are talking about all modules in general?

@bnoordhuis
Copy link
Member

@vkurchatkin Just for sys. The consensus was that things that are broken or a maintenance hassle can be phased out given enough time. For the other kind of deprecated (discouraged but not defective), it can stay in perpetuity.

@geek
Copy link
Member Author

geek commented Jan 8, 2015

@bnoordhuis thanks for the response. Your last comment cleared up what I was getting at... what the policy is for deleting deprecated items. I strongly dislike dead code, particularly because it does add cost to a project.

I'll update the PR as outlined above.

@rvagg
Copy link
Member

rvagg commented Jan 12, 2015

@geek will you be updating this or shal we open it up for others to contribute by submitting a new PR?

@geek
Copy link
Member Author

geek commented Jan 12, 2015

@rvagg I'll do it now... do you want it in the v1 branch?

@geek geek mentioned this pull request Jan 12, 2015
@geek
Copy link
Member Author

geek commented Jan 12, 2015

Closing in favor of #309

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.

None yet

7 participants