Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always quit with the correct exit code #44

Closed
wants to merge 1 commit into from

Conversation

addaleax
Copy link

Quit with the exit code of the first failing child, even if the other children exit very fast after the failing child.

Output with the current master:

$ ./index.js -v true false true ; echo $?
`exec true` ended successfully


### Status ###
`exec true` finished
`exec false` is still running
`exec true` is still running


`exec false` failed with exit code 1
`exec true` will now be closed
`exec true` will now be closed


### Status ###
`exec true` finished
`exec false` errored
`exec true` is still running


0

This patch changes the output code to 1. If you have a better solution and/or understanding of the problem, feel free to discard this PR, but the underlying issue is real.

Quit with the exit code of the first failing child, even
if the other children exit very fast after the failing child.
@keithamus
Copy link
Collaborator

Hey @addaleax thanks for the PR.

Looking at the code, I think we already do exit if the code > 0, also I think this may break the --wait feature. Sadly this is a difficult problem to solve, but we're on it - check out #30 and more importantly mysticatea/npm-run-all#10

@keithamus
Copy link
Collaborator

I think I'll close this. Thanks very much for the PR though. If you feel like helping out, once we close mysticatea/npm-run-all#10 I'll deprecate this repo and we would absolutely love to see you over in http://github.com/mysticatea/npm-run-all raising issues and making PRs 😄

@keithamus keithamus closed this Jan 28, 2016
@addaleax
Copy link
Author

Okay, thanks for the info!

@addaleax addaleax mentioned this pull request Jan 28, 2016
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants