refactor: foregroundChild arguments normalization #25
Conversation
demurgos
force-pushed
the
refactor-arguments-normalization
branch
from
6c0acb9
to
ac478ce
Compare
|
The CI failure seems to be an Appveyor issue (issue in their |
|
This also fixes a bug: #30 (comment) |
index.js
Outdated
| var lastFgArg = fgArgs[fgArgs.length - 1]; | ||
| if (typeof lastFgArg === "function") { | ||
| cb = lastFgArg; | ||
| processArgsEnd--; |
There was a problem hiding this comment.
Prefer use of -= 1 instead for explicitness.
There was a problem hiding this comment.
Should I push force the update or use an extra commit to address this?
| } else if (!Array.isArray(args)) { | ||
| args = [].slice.call(arguments, 1, arrayIndex) | ||
| } | ||
| return {program: program, args: args, cb: cb}; |
There was a problem hiding this comment.
Since we're doing ES6, this could just be:
return { program, args, cb };There was a problem hiding this comment.
Well, no ES6 for the moment. But yeah, I used object short hand originally.
I'll send a follow-up PR to move to ES6.
| * ``` | ||
| */ | ||
| module.exports = function (/* program, args, cb */) { | ||
| var fgArgs = normalizeFgArgs([].slice.call(arguments, 0)); |
There was a problem hiding this comment.
Why not just Array.from(arguments) ?
There was a problem hiding this comment.
This is what was used originally. I plan to move to ES6 later anyway, where it'll simply become function(...fgArgs).
demurgos
force-pushed
the
refactor-arguments-normalization
branch
2 times, most recently
from
4b2371c
to
244e8e1
Compare
# Why `foregroundChild` has a complex signature. Using a dedicated function for arguments normalization would improve clarity. # What Document the signature of `foregroundChild`, move arguments normalization to a separate function, small simplification to the normalization.
demurgos
force-pushed
the
refactor-arguments-normalization
branch
from
244e8e1
to
afc1466
Compare
|
@demurgos this seems reasonable to me, do we have tests already for each function signature? |
Why
foregroundChildhas a complex signature. Using a dedicated function for arguments normalization would improve clarity.What
Document the signature of
foregroundChild, move arguments normalization to a separate function, small simplification to the normalization.