Closed melaniecebula closed 9 years ago
this looks great. i was looking around for tests and didn't find any :O it might be a good idea to have them at some point
Yes, tests would be great. (That way I could know if I broke things .. :P)
Instead of piping background streams directly to stdout you should join them with the main pipeline using a parallel-multistream
before returning.
if (bkgds.length) {
return parallel(mainPipeline, bkgds)
} else {
return mainPipeline
}
That means you (the user) can pipe it anywhere you want! :)
@karissa the lack of tests are my fault - we should definitely add some soon :) @melaniecebula other than my comments this looks great!
we should probably rebase this to a single (or a few) commit as well
Awesome! I'll be incorporating your guy's feedback and update the PR later today!
:+1: LGTM from me
@melaniecebula i've added you as owner on the npm module as well so you can have the honor of releasing this :)
NOTE: Planning to add support for nested commands in another PR
Following mafintosh's proposed plan here, which I really like: https://gist.github.com/mafintosh/352f84dc924e0c404e99
Feedback appreciated!