child_process: spawn optional process arguments #6103

Closed
wants to merge 1 commit into
from

Projects

None yet

4 participants

@alexwhitman

child_process.spawn required an empty arguments array if also using options. This removes that requirement.

spawn(command, [], options) -> spawn(command, options)

Fixes #6068. The documentation already implies that args is optional so no update needed. The patch was done against master but will apply cleanly against 0.10.x if required.

@Nodejs-Jenkins

Thank you for contributing this pull request! Here are a few pointers to make sure your submission will be considered for inclusion.

The following commiters were not found in the CLA:

  • Alex Whitman

You can fix all these things without opening another issue.

Please see CONTRIBUTING.md for more information

@alexwhitman alexwhitman child_process: spawn optional process arguments
child_process.spawn required an empty arguments array if also using
options. This removes that requirement.

spawn(command, [], options) -> spawn(command, options)
c3ddd89
@alexwhitman

Updated commit with correct email address which should be in the CLA.

@sam-github
Member

I'm -2 on this, because it creates incompatibilities with the type check for missing args done by fork, and by execFile, which have the same (name, [args,] [options,]... API pattern.

I'd be +3 if it did the check in the exact same way as the other two similar APIs in child_process, either by modifying the others to do it your way, or changing your way to be the same as the others, and +6 if it introduced tests for all 3 of these APIs asserting that they behave identically in their handling of optional arguments.

@alexwhitman

I'll take a look a cleaning this up over the weekend.

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