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

chore: add shell flag to prepare script #2646

Merged
merged 1 commit into from
Dec 1, 2020

Conversation

nbbeeken
Copy link
Contributor

@nbbeeken nbbeeken commented Dec 1, 2020

This should fix the script failing to run on windows.

I had added this flag in earlier work when we wanted a more complex prepare script but we abandoned that, however we should still keep this flag to fix windows use cases.

@nbbeeken nbbeeken requested a review from emadum December 1, 2020 12:38
Copy link
Contributor

@emadum emadum left a comment

Choose a reason for hiding this comment

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

This does in fact fix it, I also made this change in #2633 but happy to merge it in separately!

One question, is there any harm to passing {shell: true} on platforms other than Windows?

@nbbeeken
Copy link
Contributor Author

nbbeeken commented Dec 1, 2020

One question, is there any harm to passing {shell: true} on platforms other than Windows?

No harm, but someone astutely made the point that it adds expense where none is needed, I was inclined to agree. I'll merge this now just to unblock those using the git link dep style, we can simplify it back to shell: true in your PR if we wish. :) ty for speedy ✅

@nbbeeken nbbeeken merged commit 98ae02c into master Dec 1, 2020
@nbbeeken nbbeeken deleted the chore/add-shell-flag-prepare branch December 1, 2020 13:01
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