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.

Sign up for GitHub

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

Jump to bottom

net: remove deprecated getters for internals #17141

Closed
wants to merge 1 commit into from
Closed

net: remove deprecated getters for internals #17141

wants to merge 1 commit into from

Conversation

addaleax
Copy link
Member

Remove the getters introduced in 75a19fb.

Refs: #14449

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

net

@addaleax
net: remove deprecated getters for internals
820bc5c 
Remove the getters introduced in 75a19fb.

Refs: nodejs#14449
@addaleax addaleax added net Issues and PRs related to the net subsystem. semver-major PRs that contain breaking changes and should be released in the next major version. labels Nov 19, 2017
@addaleax addaleax added this to the 10.0.0 milestone Nov 19, 2017
@nodejs-github-bot nodejs-github-bot added the net Issues and PRs related to the net subsystem. label Nov 19, 2017
@gireeshpunathil
Copy link
Member 

> new require('net').Server()._setupSlave
[Function]
>

Can this change break any external software that might have programmed based on this internal function?

@TimothyGu
Copy link
Member

@gireeshpunathil Merely accessing the function doesn't emit a warning, but calling it in any fashion would.

@gireeshpunathil
Copy link
Member

@TimothyGu - my question was not on emitting warnings. The function is accessible externally to user land, so will there be any modules that built dependency on the internal behavior of these routines, was my query.

@addaleax
Copy link
Member Author

I mean, yes, this is a semver-major change, that’s 100 % clear. It’s however not generally useful to userland applications and there were no complaints about the deprecation warning since it was introduced.

@gireeshpunathil
Copy link
Member

thanks @addaleax , understood

@joyeecheung
Copy link
Member

CI: https://ci.nodejs.org/job/node-test-pull-request/11596/

@joyeecheung joyeecheung added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 21, 2017
jasnell pushed a commit that referenced this pull request Nov 22, 2017
@addaleax @jasnell
net: remove deprecated getters for internals
3701b02 
Remove the getters introduced in 75a19fb.

PR-URL: #17141
Refs: #14449
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell
Copy link
Member

jasnell commented Nov 22, 2017

Landed in 3701b02

@jasnell jasnell closed this Nov 22, 2017
@addaleax addaleax removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gireeshpunathil gireeshpunathil gireeshpunathil approved these changes

@JungMinu JungMinu JungMinu approved these changes

@targos targos targos approved these changes

@TimothyGu TimothyGu TimothyGu approved these changes

@evanlucas evanlucas evanlucas approved these changes

@jasnell jasnell jasnell approved these changes

@bnoordhuis bnoordhuis bnoordhuis approved these changes

@refack refack refack approved these changes

@apapirovski apapirovski apapirovski approved these changes

Assignees
No one assigned
Labels
net Issues and PRs related to the net subsystem. semver-major PRs that contain breaking changes and should be released in the next major version.
Projects
None yet
Milestone
10.0.0
Development

Successfully merging this pull request may close these issues.