-
Notifications
You must be signed in to change notification settings - Fork 468
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
all: remove deprecated stuff but keep infra #572
Conversation
Remove all the deprecated features, but keep the infrastructure and the documentation thereof in place to allow deprecating features in the future. Re: nodejs#570
97f54ad
to
fac7d8c
Compare
Hmmm .. why is travis testing on v6.x? |
Does getting rid of Line 15 in bcc1d58
|
@cjihrig no doubt, I just don't know if we have a good reason for keeping it around 🤷 |
Hmmm ... git blame reveals no good reason. I think I'll drop it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I'm not 100% sure we want to do this. I know we are planning for a SemVer major but the other changes were very unlikely to cause any code to stop compiling/working. This removes functions that may be used. @gabrielschulhof is there a reason we should remove now? |
@mhdawson only that a semver-major release is an opportunity to drop things. I don't feel strongly about dropping the deprecated stuff. I'm OK with leaving it in until the next major. |
I think #576 should fix the CI failures on Node v6 targets. |
Closing this for now. We need not be aggressive about removing deprecated stuff until we're really stumbling over it. |
Remove all the deprecated features, but keep the infrastructure and the
documentation thereof in place to allow deprecating features in the
future.