-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
child_process: simplify argument handling #25194
Conversation
function execSync(command /* , options */) { | ||
var opts = normalizeExecArgs.apply(null, arguments); | ||
function execSync(command, options) { | ||
var opts = normalizeExecArgs(command, options, null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest to remove null
as it's not required but by passing it through explicitly it somewhat seems like that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Last I heard, it was still better to pass the exact number of expected arguments. If that has changed, I'm happy to remove the optional parameter.
This commit simplifies the calling of normalizeSpawnArguments() and normalizeExecArguments(). Specifically, this commit replaces apply() and the use of arguments with a normal function call. PR-URL: nodejs#25194 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Yellow CI run: https://ci.nodejs.org/job/node-test-pull-request/19856/ |
This commit simplifies the calling of normalizeSpawnArguments() and normalizeExecArguments(). Specifically, this commit replaces apply() and the use of arguments with a normal function call. PR-URL: #25194 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
This commit simplifies the calling of normalizeSpawnArguments() and normalizeExecArguments(). Specifically, this commit replaces apply() and the use of arguments with a normal function call. PR-URL: nodejs#25194 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
This commit simplifies the calling of normalizeSpawnArguments() and normalizeExecArguments(). Specifically, this commit replaces apply() and the use of arguments with a normal function call. PR-URL: #25194 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
This commit simplifies the calling of normalizeSpawnArguments() and normalizeExecArguments(). Specifically, this commit replaces apply() and the use of arguments with a normal function call. PR-URL: #25194 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
This commit simplifies the calling of normalizeSpawnArguments() and normalizeExecArguments(). Specifically, this commit replaces apply() and the use of arguments with a normal function call. PR-URL: #25194 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
This commit simplifies the calling of normalizeSpawnArguments() and normalizeExecArguments(). Specifically, this commit replaces apply() and the use of arguments with a normal function call. PR-URL: #25194 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
This commit simplifies the calling of
normalizeSpawnArguments()
andnormalizeExecArguments()
. Specifically, this commit replacesapply()
and the use ofarguments
with a normal function call.Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes