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: Mark as deprecated #309

Closed
wants to merge 4 commits into from
Closed

sys: Mark as deprecated #309

wants to merge 4 commits into from

Conversation

geek
Copy link
Member

@geek geek commented Jan 12, 2015

Replaces #182


var setExports = util.deprecate(function() {
module.exports = util;
}, 'sys is deprecated, use util instead');
Copy link
Contributor

Choose a reason for hiding this comment

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

Small nit, but could you change this to:

sys is deprecated. Use util instead. (Ref: #166 (comment))

@cjihrig
Copy link
Contributor

cjihrig commented Jan 12, 2015

Other than one small comment, LGTM

@cjihrig
Copy link
Contributor

cjihrig commented Jan 12, 2015

This falls into the same category as the fs.exists() deprecation in #257. Should this wait until after 1.0.0? cc: @piscisaureus

@cjihrig
Copy link
Contributor

cjihrig commented Jan 12, 2015

@geek this fails make lint:

File does not end with new line. (setExports();)

@geek
Copy link
Member Author

geek commented Jan 12, 2015

@cjihrig fixed

@rvagg
Copy link
Member

rvagg commented Jan 12, 2015

I think this should land for 1.0.0, it was discussed and agreed at a TC meeting and was just waiting for the code. So lgtm.

@rvagg
Copy link
Member

rvagg commented Jan 12, 2015

@geek please squash the commits into the original one and we should be able to land this I think

@geek
Copy link
Member Author

geek commented Jan 13, 2015

Closing in favor of #317

@geek geek closed this Jan 13, 2015
@geek
Copy link
Member Author

geek commented Jan 13, 2015

@rvagg fixed and ready in #317

@geek geek deleted the v1.x branch January 13, 2015 00:43
@rvagg rvagg mentioned this pull request Jan 13, 2015
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.

3 participants