Skip to content

net: remove deprecated getters for internals #17141

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
Closed
addaleax wants to merge 1 commit into from
Closed

net: remove deprecated getters for internals #17141

addaleax wants to merge 1 commit into from

Conversation

@addaleax
Copy link
Member

@addaleax addaleax commented Nov 19, 2017

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

gireeshpunathil commented Nov 20, 2017 

> 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

TimothyGu commented Nov 20, 2017

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

@gireeshpunathil
Copy link
Member

gireeshpunathil commented Nov 20, 2017

@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

addaleax commented Nov 20, 2017

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

gireeshpunathil commented Nov 20, 2017

thanks @addaleax , understood

@joyeecheung
Copy link
Member

joyeecheung commented Nov 21, 2017

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

@bnoordhuis bnoordhuis bnoordhuis approved these changes

@jasnell jasnell jasnell approved these changes

@targos targos targos approved these changes

@JungMinu JungMinu JungMinu approved these changes

@gireeshpunathil gireeshpunathil gireeshpunathil approved these changes

+4 more reviewers

@apapirovski apapirovski apapirovski approved these changes

@refack refack refack approved these changes

@evanlucas evanlucas evanlucas approved these changes

@TimothyGu TimothyGu TimothyGu approved these changes

Reviewers whose approvals may not affect merge requirements

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.