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

Include killed boolean in command result #250

Merged
merged 4 commits into from
May 9, 2021

Conversation

Roaders
Copy link
Contributor

@Roaders Roaders commented Nov 23, 2020

The exit code of each command is already included in the result but the usefulness of this is limited if concurrently is run with killOthers. When parsing the result you do not know if a non-0 exit code is because the command failed or if it was killed by concurrently.

Adding this option allows you to print a message at the end identifying exactly which commands caused the initial failure.

@coveralls
Copy link

coveralls commented Nov 23, 2020

Coverage Status

Coverage decreased (-0.3%) to 99.747% when pulling 67e677d on Roaders:master into aad79fa on kimmobrunfeldt:master.

@Roaders
Copy link
Contributor Author

Roaders commented Feb 23, 2021

Any chance we can get this PR considered?

Copy link
Member

@gustavohenke gustavohenke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey! Can we also get some test for this, please?

@Roaders
Copy link
Contributor Author

Roaders commented May 9, 2021

As far as I can tell there are no existing tests that test the exit condition of multiple commands in the close stream. I am trying to find one to base my tests on.

@Roaders
Copy link
Contributor Author

Roaders commented May 9, 2021

OK, I figured it out. Been so long since I did this I had forgotten the structure of the project.

@Roaders Roaders requested a review from gustavohenke May 9, 2021 08:39
Copy link
Member

@gustavohenke gustavohenke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the side fixes!

src/command.spec.js Outdated Show resolved Hide resolved
@Roaders
Copy link
Contributor Author

Roaders commented May 9, 2021

Do I have to do anything to merge this? I assume you'll merge it at some point.
Thanks.

@gustavohenke gustavohenke merged commit d38ab32 into open-cli-tools:master May 9, 2021
@Roaders
Copy link
Contributor Author

Roaders commented May 24, 2021

Any idea when this will be released?

@gustavohenke
Copy link
Member

Heya. Just released it in v6.2.0.

@Roaders
Copy link
Contributor Author

Roaders commented May 24, 2021

brilliant, thanks

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.

3 participants