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

Update travis and shippable to have less needless cruft #12553

Merged
merged 1 commit into from
Aug 12, 2015

Conversation

eparis
Copy link
Contributor

@eparis eparis commented Aug 11, 2015

It looks like there are places where things needed for one command
were being just needlessly copied. Which made the files harder to
recognize what mattered and what didn't...

@eparis eparis force-pushed the travis-shippable-cleanup branch from 0f99c2c to 0726ef3 Compare August 11, 2015 21:33
It looks like there are places where things were needed for one command
were being just needlessly copied. Which made the files harder to
recognize what mattered and what didn't...
@eparis eparis force-pushed the travis-shippable-cleanup branch from 0726ef3 to df98870 Compare August 11, 2015 21:43
@eparis
Copy link
Contributor Author

eparis commented Aug 11, 2015

@mikedanese since you are looking at these guys.

@eparis eparis changed the title WIP: Update travis and shippable to have less needless cruft Update travis and shippable to have less needless cruft Aug 11, 2015
@mikedanese mikedanese self-assigned this Aug 11, 2015
@mikedanese
Copy link
Member

LGTM. Thanks!

@k8s-bot
Copy link

k8s-bot commented Aug 11, 2015

GCE e2e build/test passed for commit df98870.

@mikedanese mikedanese added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 12, 2015
cjcullen added a commit that referenced this pull request Aug 12, 2015
Update travis and shippable to have less needless cruft
@cjcullen cjcullen merged commit 72ff117 into kubernetes:master Aug 12, 2015
@eparis eparis deleted the travis-shippable-cleanup branch August 12, 2015 01:16
@uluyol
Copy link
Contributor

uluyol commented Aug 12, 2015

What's the reason for removing these? I don't see what's so harmful about having extraneous PATH entries. I'm all for cleaning up our bash scripts and test config, but it seems strange to provide different scripts with different environments.

This subtly breaks shippable when combined with #12411, and whether you agree with the decision to add a new build dep in that PR or not, I don't think we want surprises like this in our test config.

@eparis
Copy link
Contributor Author

eparis commented Aug 12, 2015

These definitely need "undone" for deep-copy and conversions. If we want consistency across all scripts that's fine. But that means we should just explicitly export the PATH for everyone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants