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

Only shift once, not twice #5552

Merged
merged 1 commit into from Mar 7, 2018

Conversation

Projects
None yet
3 participants
@illicitonion
Copy link
Contributor

illicitonion commented Mar 6, 2018

cb72c2a removed the CORE_ONLY which was
the second shift target.

It turns out that bash no-ops if you shift 2 when there is only one
arg. So that's fun.

Also, set -e for this script. Note that it doesn't actually do
anything about the shift error (that would be far too useful), but
it's something we should generally be doing.

But also, we should rewrite this script in python or something, because
it's gotten insane. Filed
#5551 to track that.

@illicitonion illicitonion requested a review from stuhood Mar 6, 2018

Only shift once, not twice
cb72c2a removed the CORE_ONLY which was
the second shift target.

It turns out that bash no-ops if you `shift 2` when there is only one
arg. So that's fun.

Also, `set -e` for this script. Note that it doesn't actually do
anything about the `shift` error (that would be far too useful), but
it's something we should generally be doing.

But also, we should rewrite this script in python or something, because
it's gotten insane. Filed
#5551 to track that.

@illicitonion illicitonion force-pushed the twitter:dwagnerhall/releasescript branch from 6d1eda9 to a24c85a Mar 6, 2018

@stuhood

stuhood approved these changes Mar 7, 2018

@illicitonion illicitonion merged commit e1cd9d4 into pantsbuild:master Mar 7, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@illicitonion illicitonion deleted the twitter:dwagnerhall/releasescript branch Apr 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment