Skip to content

Evaluate glide-hash result without reading $?.#1214

Merged
timoreimann merged 1 commit intotraefik:masterfrom
timoreimann:avoid-validate-glide-output-suppression
Mar 2, 2017
Merged

Evaluate glide-hash result without reading $?.#1214
timoreimann merged 1 commit intotraefik:masterfrom
timoreimann:avoid-validate-glide-output-suppression

Conversation

@timoreimann
Copy link
Contributor

@timoreimann timoreimann commented Mar 1, 2017

validate-glide is called with errexit enabled (in script/make.sh that sources validate-glide), which means that grep returning a non-zero exit code will cause the script to terminate prematurely. Thus, we will never get to the point where we see the error message.

The fix is to embed the grep check directly inside the if statement.

@timoreimann timoreimann force-pushed the avoid-validate-glide-output-suppression branch 2 times, most recently from 90214b7 to 3108ac7 Compare March 2, 2017 09:24
@timoreimann
Copy link
Contributor Author

I added a bogus, manual change to the glide.yml file to demonstrate that the error message is now shown (see Travis CI output) and reverted it afterwards. Post-LGTM, I can either squash manually or we use Github's "squash and merge" feature to keep the Git history clean.

@containous/traefik please take a look :)

Copy link
Contributor

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

LGTM 🐸

validate-glide is called with errexit enabled (in script/make.sh that
sources validate-glide), which means that grep returning a non-zero exit
code will cause the script to terminate prematurely. Thus, we will never
get to the point where we see the error message.

The fix is to embed the grep check directly inside the if statement.
@timoreimann timoreimann force-pushed the avoid-validate-glide-output-suppression branch from 3108ac7 to 04a1ecc Compare March 2, 2017 17:49
@trecloux
Copy link
Contributor

trecloux commented Mar 2, 2017

LGTM 👍

Copy link
Member

@emilevauge emilevauge left a comment

Choose a reason for hiding this comment

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

Oops 🤦‍♂️
LGTM
thanks @timoreimann

@timoreimann timoreimann merged commit a3beec6 into traefik:master Mar 2, 2017
@timoreimann timoreimann deleted the avoid-validate-glide-output-suppression branch March 2, 2017 22:00
@ldez ldez modified the milestone: 1.3 May 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants