search

coreos / fleet

fleet ties together systemd and etcd into a distributed init system
Apache License 2.0
2.42k stars 302 forks source link

Add fleet grpc vendoring #1657

Closed hectorj2f closed 8 years ago

hectorj2f commented 8 years ago

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 commented 8 years ago

ping @dongsupark

dongsupark commented 8 years ago

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 commented 8 years ago

@dongsupark Solved!. I added them to the original PR https://github.com/coreos/fleet/pull/1426

dongsupark commented 8 years ago

@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 commented 8 years ago

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.