Skip to content
This repository was archived by the owner on Jan 30, 2020. It is now read-only.

Conversation

@hectorj2f
Copy link
Contributor

These Go vendoring is related to the PR #1426. As discussed in #1426 (comment), it should be better to include the go packages in a different PR, instead of in the same PR #1426 .

@hectorj2f
Copy link
Contributor Author

ping @dongsupark

@dongsupark
Copy link

dongsupark commented Aug 10, 2016

Great! In general it looks good. I just manually checked out both #1426 and #1657 to combine into a single branch, and ran both unit tests and functional tests. It works without any error.
One problem is that glide.lock and glide.yaml in the both PRs make conflicts with each other. So I had to fix it manually. Before merging the 2 PRs, we need to sort out this part to get the glide changes included in only one of the 2 PRs.
Apart from that, I think it's ready to be merged.
Thanks! :-)

@hectorj2f hectorj2f force-pushed the fleet_grpc_vendoring branch from 4c5d545 to 38283f0 Compare August 10, 2016 19:03
@hectorj2f
Copy link
Contributor Author

@dongsupark Solved!. I added them to the original PR #1426

@dongsupark
Copy link

@hectorj2f LGTM.
I'll create another PR just for running functional tests on semaphoreci.
If that passes, I'd like to merge #1426, #1657, and #1656 tomorrow.

@dongsupark
Copy link

It turns out that travis doesn't like a test directory which is not buildable, while a unit test runs well when tested locally. Strange...

no buildable Go source files in /home/travis/gopath/src/github.com/coreos/fleet/vendor/google.golang.org/grpc/test"

I'll create a new PR for removing the vendor/google.golang.org/grpc/test, as well as adding missing license headers etc.

@dongsupark dongsupark merged commit 6ada5a0 into coreos:master Aug 12, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants