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.

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

path: replace assertPath() with validator #24840

Closed
wants to merge 1 commit into from
Closed

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Dec 4, 2018

The path module's assertPath() does exactly what the validateString() validator does, so this commit updates path to use validateString() instead. A couple drive by updates to validateString() outside of assertPath() are also included.

Technically, the validateString(from, 'from'); and validateString(to, 'to'); changes make the error message different (changes 'path' to 'to'/'from'), but these are already using the new error system, so they shouldn't be considered semver major IMO.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

The path module's assertPath() does exactly what the
validateString() validator does, so this commit updates
path to use validateString() instead. A couple drive by
updates to validateString() outside of assertPath() are
also included.
@nodejs-github-bot nodejs-github-bot added the path Issues and PRs related to the path subsystem. label Dec 4, 2018
@cjihrig
Copy link
Contributor Author

cjihrig commented Dec 6, 2018

@Trott Trott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 6, 2018
@Trott
Copy link
Member

Trott commented Dec 7, 2018

Landed in f962f97

@Trott Trott closed this Dec 7, 2018
Trott pushed a commit to Trott/io.js that referenced this pull request Dec 7, 2018
The path module's assertPath() does exactly what the
validateString() validator does, so this commit updates
path to use validateString() instead. A couple drive by
updates to validateString() outside of assertPath() are
also included.

PR-URL: nodejs#24840
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
MylesBorins pushed a commit that referenced this pull request Dec 7, 2018
The path module's assertPath() does exactly what the
validateString() validator does, so this commit updates
path to use validateString() instead. A couple drive by
updates to validateString() outside of assertPath() are
also included.

PR-URL: #24840
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
@cjihrig cjihrig deleted the path branch December 9, 2018 17:37
@BethGriggs BethGriggs mentioned this pull request Dec 18, 2018
refack pushed a commit to refack/node that referenced this pull request Jan 14, 2019
The path module's assertPath() does exactly what the
validateString() validator does, so this commit updates
path to use validateString() instead. A couple drive by
updates to validateString() outside of assertPath() are
also included.

PR-URL: nodejs#24840
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
BethGriggs pushed a commit that referenced this pull request Feb 12, 2019
The path module's assertPath() does exactly what the
validateString() validator does, so this commit updates
path to use validateString() instead. A couple drive by
updates to validateString() outside of assertPath() are
also included.

PR-URL: #24840
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
@BethGriggs BethGriggs mentioned this pull request Feb 12, 2019
BethGriggs pushed a commit that referenced this pull request Feb 20, 2019
The path module's assertPath() does exactly what the
validateString() validator does, so this commit updates
path to use validateString() instead. A couple drive by
updates to validateString() outside of assertPath() are
also included.

PR-URL: #24840
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
rvagg pushed a commit that referenced this pull request Feb 28, 2019
The path module's assertPath() does exactly what the
validateString() validator does, so this commit updates
path to use validateString() instead. A couple drive by
updates to validateString() outside of assertPath() are
also included.

PR-URL: #24840
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. path Issues and PRs related to the path subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants