This repository was archived by the owner on Oct 7, 2020. It is now read-only.
Repository navigation
This repository was archived by the owner on Oct 7, 2020. It is now read-only.
[Converge] child_process argument type checking #22
Description
These commits add argument type checking to methods within child_process. This needs to be reconciled with the current io.js behavior. The change introduces a new throw so there's a potential API compatibility issue.
/cc @trevnorris
a1b2875afd9affd3e0147a10f04a2daf8c598761 lint: fix lint issues- nodejs/node-v0.x-archive@a1b28758032a210250c86877898331ac4474f39b3a573c6 test: test all spawn parameter positions- nodejs/node-v0.x-archive@8032a2170dafa7b624abd43432e03304d65cc527fbecc11 child_process: check fork args is an array- nodejs/node-v0.x-archive@70dafa7e17c5a72b23f920f291d61f2780068c18768cb92 child_process: check execFile args is an array- nodejs/node-v0.x-archive@e17c5a7
Activity
sam-github commentedon May 21, 2015
@piscisaureus I think these are the commits you said you didn't bring over. My primary involvement in this was to make the code actually match the docs in terms of what args were or were not optional. But that fell into a can of worms because some vocal users wanted very specific type checking on the args. Which isn't a bad thing, but with four args, most optional, it gets a bit hairy. I leave it to you all to decide what to do about this.
And note that the originally claimed "API incompatiblity" was a LACK of a throw...
sam-github commentedon May 21, 2015
I think at least the tests should be ported over, since they demonstrate the very wide variety of calling conventions that are possible... what the correct behaviour should be for each one can then be discussed more readily.
jasnell commentedon May 22, 2015
@nodejs/tsc ... this may be one we need broader review on.
bnoordhuis commentedon May 27, 2015
The changes look reasonable to me. If one of you turns this into a PR and cc's me, I'll sign off on it.
jasnell commentedon May 27, 2015
I'll be working on doing so
rvagg commentedon Aug 24, 2015
now active @ nodejs/node#2515