lib: extract validateString validator #22101
Conversation
This comment has been minimized.
This comment has been minimized.
nodejs-github-bot
added
the
lib / src
There was a problem hiding this comment.
What about trying to create another new function called "isEmptyString" to check whether your string value is empty:
function isEmptyString (s){
return (typeof s === 'string' && s.trim() === '');
}
Most of your functions requires a param value that isn't null or empty.
Maybe when both of the things meet, we can continue going on.
@Maledong Not sure if this is the case -- where do you see this check? I'd rather not add new validations in this PR, since that would be a |
|
@Maledong Arguably |
lib/net.js
Outdated
| if (typeof path !== 'string') { | ||
| throw new ERR_INVALID_ARG_TYPE('options.path', 'string', path); | ||
| } | ||
| validateString(path, 'path'); |
There was a problem hiding this comment.
Name changed from "options.path" to "path". I'm on mobile so can't check which one is correct.
There was a problem hiding this comment.
I think changing the name was a mistake, but going to put this back to options.path instead of path to make sure this is definitely not semver-major :)
|
CI again: https://ci.nodejs.org/job/node-test-pull-request/16182/ (Didn't use Resume Build because windows-fanned failed and Resume Build does not cooperate with windows-fanned.) |
|
Still LGMT. |
Pulls out a common argument validator to `internal/validators`
maclover7
force-pushed
the
jm-validatestring
branch
from
67d8a5d
to
fc8d944
Compare
|
Landed in e570ae7, thank you for the reviews! |
Pulls out a common argument validator to `internal/validators` PR-URL: #22101 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Pulls out a common argument validator to `internal/validators` PR-URL: #22101 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#24863 Refs: nodejs#22101 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
PR-URL: nodejs#24960 Refs: nodejs#22101 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: nodejs#24863 Refs: nodejs#22101 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
PR-URL: nodejs#24960 Refs: nodejs#22101 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Pulls out a common argument validator to
internal/validatorsChecklist
make -j4 test(UNIX), orvcbuild test(Windows) passes