Skip to content

lib: refactor isLegalPort, move to validators - #31851

Closed
jasnell wants to merge 1 commit into
nodejs:masterfrom
jasnell:refactor-islegalport
Closed

lib: refactor isLegalPort, move to validators#31851
jasnell wants to merge 1 commit into
nodejs:masterfrom
jasnell:refactor-islegalport

Conversation

@jasnell

@jasnelljasnell commented Feb 18, 2020

Copy link
Copy Markdown
Member

isLegalPort was used multiple places in the same way -- to validate
the port and throw if necessary. Moved into internal/validators.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

@nodejs-github-botnodejs-github-bot added the lib / src Issues and PRs related to general changes in the lib or src directory. label Feb 18, 2020
jasnell
jasnell commented Feb 18, 2020
View reviewed changes
Comment threadlib/internal/validators.js Outdated
BridgeAR
BridgeAR reviewed Feb 18, 2020
View reviewed changes
Comment threadlib/dgram.js Outdated
BridgeAR
BridgeAR reviewed Feb 19, 2020
View reviewed changes
Comment threadlib/internal/validators.js Outdated
@nodejs-github-bot

nodejs-github-bot commented Mar 4, 2020

Copy link
Copy Markdown
Collaborator

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

@jasnelljasnell added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 5, 2020
@jasnell
lib: move isLegalPort to validators, refactor
5c32715
isLegalPort was used multiple places in the same way -- to validate
the port and throw if necessary. Moved into internal/validators.
@jasnell
jasnellforce-pushed the refactor-islegalport branch from a100a54 to 5c32715CompareMarch 5, 2020 19:51
@jasnell

jasnell commented Mar 5, 2020

Copy link
Copy Markdown
MemberAuthor

Landed in b023d61

@jasnelljasnell closed this Mar 5, 2020
@MylesBorins

MylesBorins commented Mar 9, 2020

Copy link
Copy Markdown
Contributor

This doesn't land cleanly on v13.x

Should it be backported?

@MylesBorins

MylesBorins commented Mar 10, 2020

Copy link
Copy Markdown
Contributor

after some other backports landed this now lands cleanly!

@MylesBorinsMylesBorins mentioned this pull request Mar 10, 2020
Merged
@HarshithaKPHarshithaKP mentioned this pull request Apr 13, 2020
Closed
3 tasks
@cjihrigcjihrig mentioned this pull request Apr 15, 2020
Closed
2 tasks
@targostargos added backport-blocked-v12.x and removed author ready PRs that have at least one approval, no pending requests for changes, and a CI started. backport-blocked-v12.x labels Apr 20, 2020
@targostargos mentioned this pull request May 2, 2020
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@BridgeARBridgeARBridgeAR left review comments
@richardlaurichardlaurichardlau approved these changes

Assignees

No one assigned

Labels

lib / srcIssues and PRs related to general changes in the lib or src directory.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@jasnell@nodejs-github-bot@MylesBorins@richardlau@BridgeAR@targos