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

fix(cli): re-enable parallel scripts #30

Merged
merged 1 commit into from Jul 8, 2016
Merged

fix(cli): re-enable parallel scripts #30

merged 1 commit into from Jul 8, 2016

Conversation

kentcdodds
Copy link
Collaborator

What:

Fixes an issue with parallel scripts

Why:

PR #29 broke the following case:

p-s -p lint,test

How:

This commit fixes it by getting the scripts from the args first and seeing if there are any scripts available to run. If not, then we show the help message.

CC: @giladgo, @abhishekisnot, @rowanoulton, @tleunen, @jisaacks, @nkbt

Could one of you give this the green light and press the green button please?

@@ -15,7 +15,7 @@ before_install:
before_script:
- npm prune
script:
- npm start validate
- npm run localstart validate
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is temporary. Because p-s uses itself, it is also broken by the thing this PR is fixing.

@kentcdodds
Copy link
Collaborator Author

:shipit:

@codecov-io
Copy link

codecov-io commented Jul 8, 2016

Current coverage is 100%

No coverage report found for master at 54b0f8b.

Powered by Codecov. Last updated by 54b0f8b...27e2843

@kentcdodds kentcdodds force-pushed the pr/fix-parallel branch 2 times, most recently from 88193c4 to 453ba52 Compare July 8, 2016 17:53
@@ -82,6 +82,10 @@
"lcov",
"text",
"html"
],
"exclude": [
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This stuff is actually in master right now, not sure why it's still showing up in this PR. Ignore it :-)

@kentcdodds
Copy link
Collaborator Author

Ok, I'm pretty certain this'll work now :-)

PR #29 broke the following case:

```
p-s -p lint,test
```

This commit fixes it by getting the scripts from the args first and
seeing if there are any scripts available to run. If not, then we show
the help message.
@kentcdodds
Copy link
Collaborator Author

Sweet! The build is finally green!

Pinging again: @giladgo, @abhishekisnot, @rowanoulton, @tleunen, @jisaacks, @nkbt

Could one of you review this and merge if it looks good?

@tleunen
Copy link
Collaborator

tleunen commented Jul 8, 2016

LGTM ;)

@tleunen tleunen merged commit ec36b75 into master Jul 8, 2016
@tleunen tleunen deleted the pr/fix-parallel branch July 8, 2016 19:41
@nkbt
Copy link
Collaborator

nkbt commented Jul 8, 2016

Nice.
Just got greenkeeper updates. All builds are green :)

@kentcdodds
Copy link
Collaborator Author

Thanks for the update @nkbt!

@kentcdodds kentcdodds mentioned this pull request Jul 9, 2016
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.

None yet

4 participants