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 an unused internal module assertPort #11812

Closed
wants to merge 1 commit into from
Closed

net: remove an unused internal module assertPort #11812

wants to merge 1 commit into from

Conversation

watilde
Copy link
Member

@watilde watilde commented Mar 12, 2017

Since a module assertPort in lib/internal/net.js is not used anymore, we can remove it. The coverage of lib/internal/net.js will be 100% with this.

See also: #11667

Checklist
  • make -j4 test
  • commit message follows commit guidelines
Affected core subsystem(s)

net

@watilde
net: remove an unused internal module assertPort
23587b6 
A module `assertPort` in `lib/internal/net.js` is not used anymore.

Refs: nodejs#11667
@nodejs-github-bot nodejs-github-bot added the net Issues and PRs related to the net subsystem. label Mar 12, 2017
@JacksonTian
Copy link
Contributor

LGTM

@mscdex
Copy link
Contributor

mscdex commented Mar 12, 2017

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

@watilde
Copy link
Member Author

watilde commented Mar 13, 2017

Probably, the CI errors caused by the same reason with this: #11817 (comment). I will retry it.

@watilde
Copy link
Member Author

watilde commented Mar 13, 2017

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

@watilde watilde mentioned this pull request Mar 13, 2017
Closed
3 tasks
@watilde
Copy link
Member Author

watilde commented Mar 14, 2017

Landed in 879d666

@watilde watilde closed this Mar 14, 2017
@watilde watilde deleted the feature/assertPort branch March 14, 2017 17:57
watilde added a commit that referenced this pull request Mar 14, 2017
@watilde
net: remove an unused internal module assertPort
879d666 
A module `assertPort` in `lib/internal/net.js` is not used anymore.

Refs: #11667
PR-URL: #11812
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
jungx098 pushed a commit to jungx098/node that referenced this pull request Mar 21, 2017
@watilde @jungx098
net: remove an unused internal module assertPort
7f8cd16 
A module `assertPort` in `lib/internal/net.js` is not used anymore.

Refs: nodejs#11667
PR-URL: nodejs#11812
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
@italoacasas
Copy link
Contributor

This PR needs backport to v7

@jasnell jasnell mentioned this pull request Apr 4, 2017
Closed
watilde added a commit to watilde/node that referenced this pull request May 5, 2017
@watilde
net: remove an unused internal module assertPort
acb77f3 
A module `assertPort` in `lib/internal/net.js` is not used anymore.

Refs: nodejs#11667
PR-URL: nodejs#11812
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
@watilde watilde mentioned this pull request May 5, 2017
Closed
2 tasks
watilde added a commit that referenced this pull request May 12, 2017
@watilde
net: remove an unused internal module assertPort
33ea7a2 
A module `assertPort` in `lib/internal/net.js` is not used anymore.

Refs: #11667
PR-URL: #11812
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Evan Lucas <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
@gibfahn gibfahn mentioned this pull request Jun 15, 2017
Closed
3 tasks
@gibfahn
Copy link
Member

gibfahn commented Jun 17, 2017

@watilde should this be backported to v6.x? 33ea7a2 applies cleanly, so if it should I'll cherry-pick it directly.

@MylesBorins
Copy link
Contributor

ping @watilde

1 similar comment
@MylesBorins
Copy link
Contributor

ping @watilde

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasnell jasnell jasnell approved these changes

@evanlucas evanlucas evanlucas approved these changes

@addaleax addaleax addaleax approved these changes

@lpinca lpinca lpinca approved these changes

@targos targos targos approved these changes

@cjihrig cjihrig cjihrig approved these changes

@joyeecheung joyeecheung joyeecheung approved these changes

Assignees
No one assigned
Labels
net Issues and PRs related to the net subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.