abort all processes when one fails #53
Conversation
| child.on('exit', exitCode => cb(null, exitCode)) | ||
| } else { | ||
| cb(child) | ||
| if (options.parallel) { |
There was a problem hiding this comment.
I feel like the code below could be greatly simplified with RxJS. But I have no idea how to use Observables. Someone please help me!
There was a problem hiding this comment.
I don't really see an opportunity for observables. Might be missing something though. Why do you think observables make sense here?
There was a problem hiding this comment.
I've never used Observables, but I thought that they're supposed to really help with complex async stuff. 🤷
There was a problem hiding this comment.
I think it can be done a little bit cleaner though. I'll do a PR later today or this weekend.
There was a problem hiding this comment.
Maybe zip is what you're looking for (re RxJS)? Merge multiple Observables/Promises, subscribe to the result and have a single handling of their results ...
There was a problem hiding this comment.
I don't know what I'm looking for.. I just think this code could be cleaned up a bit...
There was a problem hiding this comment.
When running in parallel, you can just use Promise.all, when executing serially you can do something like this.
const result = promises.reduce((res, task) => {
return res.then(() => task());
}, Promise.resolve());
result.then(...);
kentcdodds
force-pushed
the
pr/async-kill
branch
from
359b748 to
97d7f47
Compare
Current coverage is 100% (diff: 100%)@@ master #53 diff @@
===================================
Files 11 11
Lines 217 242 +25
Methods 0 0
Messages 0 0
Branches 0 0
===================================
+ Hits 217 242 +25
Misses 0 0
Partials 0 0
|
|
Sweet I think this is good to ship Could I get some people to test it? I've released it as the beta: cc @boneskull, @tleunen, @nkbt, @DavidWells, @Hypercubed, @rowanoulton This is a significant refactor and I'd hate to break anyone due to this bug fix! (Ref #48) |
This is a significant refactor to make it so when an error occurs in one parallel script, the others are killed and also to ensure that an error in one serial script will result in the others not being executed. Closes #48
kentcdodds
force-pushed
the
pr/async-kill
branch
from
3e01c7f to
c911c67
Compare
kentcdodds
changed the title
|
|
|
This is good to merge by anyone as soon as it's reviewed! |
| import Promise from 'bluebird' | ||
| import colors from 'colors/safe' | ||
| import {isString, find, clone} from 'lodash' | ||
| import {isString, clone} from 'lodash' |
There was a problem hiding this comment.
Full lodash is quite huge, why not install them separately?
There was a problem hiding this comment.
I get you, but honestly I'm using about a dozen methods and I'd rather avoid installing each separately/managing them. It's 1.4MB (uncompressed) and the likelihood that someone's already depending on lodash in their project (even if it's transitively) is quite high.
Thanks for the suggestion.
|
I gave it a quick run on OSX and windows. No issues other than an unrelated issue on windows (where you aware that the command So... LGTM. |
Oh no! I wasn't. Could you file an issue for that? Thanks for testing it out @Hypercubed! |
This updates the contributions for: - @kentcdodds - @tleunen - @Hypercubed It adds the following new contributors: - @jisaacks - @boneskull - @RobinMalfait - @edm00se - @SamVerschueren Thanks for the contributions everyone!
|
|
|
haha, I need more than 22h notice 😉 |
|
I guess I was anxious and excited :-) |
|
Have you considered therapy? 😉 |
ref #48