vendor: add missing deps and remove go get in .travis.yml#415
vendor: add missing deps and remove go get in .travis.yml#415crosbymichael merged 1 commit intocontainerd:masterfrom
go get in .travis.yml#415Conversation
|
It might be worth removing the |
ea6ead6 to
1719b33
Compare
go get in .travis.yml
|
Yes, I was also thinking of that. Updated PR. |
Signed-off-by: Akihiro Suda <suda.akihiro@lab.ntt.co.jp>
1719b33 to
4a7a8ef
Compare
| github.com/opencontainers/runtime-spec v1.0.0-rc3 | ||
| # logrus, latest release as of 12/16/2016 | ||
| # FIXME: github.com/docker/docker/pkg/archive requires this pkg as github.com/Sirupsen/logrus. | ||
| # So we vendor github.com/Sirupsen as well. |
There was a problem hiding this comment.
I'll open another PR to fix this FIXME later
There was a problem hiding this comment.
@crosbymichael opened containerd/go-runc#5, which replaces sirupsen with Sirupsen.
Another idea is to replace Sirupsen with sirupsen, but it would require modifying many files in github.com/docker/docker
So I suggest replacing sirupsen with Sirupsen.
There was a problem hiding this comment.
I think it's better to update docker/docker, the name is never going back to have a capital S.
There was a problem hiding this comment.
I agree that we must get all vendored-in content to the new name. However, just to frame that it may not be exceedingly quick to do so [although the problem here in this project may be limited to a few pkg/* files?]:
193 files in docker/docker need the capitalization change.. fairly easy PR to create, and I'm willing to do so over there.
But, the following vendors (at least at the version imported to docker/docker today) also are using capital-S Sirupsen:
docker/distribution
docker/go-connections
docker/go-events
docker/libnetwork
docker/notary
docker/swarmkit
opencontainer/runc
Microsoft/hcsshim
Azure/go-ansiterm
So, each of those also need a PR merged and then a re-vendor (or just a re-vendor in case the master of those projects has fixed this since the last vendor update to docker/docker).
There was a problem hiding this comment.
Looking at https://github.com/docker/containerd/issues/376#issuecomment-267718697 , probably requirement for pkg/archive will be removed (in Phase 2 or 3?). Then this sirupsen-Sirupsen issue will be solved without changes for docker/docker.
|
Thanks @AkihiroSuda for catching the missing vendors; I think the Sirupsen change is the best we can do until all dependencies fix their imports to the new lowercase sirupsen. |
|
LGTM |
Signed-off-by: Akihiro Suda suda.akihiro@lab.ntt.co.jp