-
Notifications
You must be signed in to change notification settings - Fork 1k
hack/validate-vendor.bash: fix build dep & script without TRAVIS_BRANCH #369
Conversation
hack/validate-vendor.bash
Outdated
go build | ||
./dep ensure | ||
go install ./cmd/dep | ||
dep ensure |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Depending on how PATH is setup, this isn't guaranteed to use the binary we just built. Can we fix the build command and continue using ./dep
?
hack/validate-vendor.bash
Outdated
|
||
IFS=$'\n' | ||
files=( $(git diff "$TRAVIS_BRANCH...$VALIDATE_HEAD" --diff-filter=ACMR --name-only -- 'manifest.json' 'lock.json' 'vendor/' || true) ) | ||
files=( $(validate_diff --diff-filter=ACMR --name-only -- 'manifest.json' 'lock.json' 'vendor/' || true) ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since #342 was just merged, this should be updated to manifest.json -> Gopkg.toml
and lock.json -> Gopkg.lock
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yay! 🎉 So happy to have this fixed to work locally.
b3f1bdd
to
33cddec
Compare
updated, thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Life goal of reviewing a role model's code accomplished!
omg 😍 I am not that cool you will soon see, but <3 |
😛 Whatevs, you cannot stop my fangirl squee power. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
confoozled
hack/validate-vendor.bash
Outdated
unset IFS | ||
|
||
if [ ${#files[@]} -gt 0 ]; then | ||
# We run ensure to and see if we have a diff afterwards | ||
go build | ||
go install ./cmd/dep |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
err wait i'm confused, won't this still install to GOPATH/bin
, rather than the current dir? shouldn't it be go build ./cmd/dep
?
You are correct good catch I changed the other back and not this one...
Will update
…On Apr 13, 2017 20:13, "sam boyer" ***@***.***> wrote:
***@***.**** commented on this pull request.
confoozled
------------------------------
In hack/validate-vendor.bash
<#369 (comment)>:
> unset IFS
if [ ${#files[@]} -gt 0 ]; then
# We run ensure to and see if we have a diff afterwards
- go build
+ go install ./cmd/dep
err wait i'm confused, won't this still install to GOPATH/bin, rather
than the current dir? shouldn't it be go build ./cmd/dep?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#369 (review)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ABYNbO5VDEVfBoPRJCjwcQs92ROHBlr2ks5rvroXgaJpZM4M5GuO>
.
|
Signed-off-by: Jess Frazelle <acidburn@google.com>
OK I think we're good now, mergey merge time |
hack/validate-vendor.bash: fix build dep & script without TRAVIS_BRANCH
This removes
TRAVIS_BRANCH
so the script works the same locally as in Travis. Also fixes "build dep" to run ensure since the command code was moved tocmd/dep
.