Skip to content

Conversation

@xiekeyang
Copy link
Owner

Signed-off-by: xiekeyang [email protected]

@xiekeyang xiekeyang force-pushed the vendor branch 2 times, most recently from 4acfdc1 to 863f109 Compare September 20, 2017 09:00
@wking
Copy link
Contributor

wking commented Sep 20, 2017

Instead of adding these under tools/vendor, I'd rather have them under tools/cmd/vendor. That way the library packages will not use locally-vendored code (which can lead to problems, see opencontainers/image-spec#528 and especially this comment).

It would be nice to have the Makefile's Go-testing targets temporarily link tools/cmd/vendor to tools/vendor for reliable tests, but that can happen as a second phase.

The packages we depend on also seem fairly stable, so I'm also ok if we want to punt on vendoring for now and only add it if/when we get more build/install tooling around our installed command. When we land vendoring, it would be good to also land docs on how we expect the vendor directory to be maintained. I'll expect we want to use some tool like Godep, Glide, etc.

@xiekeyang
Copy link
Owner Author

So I move vendor to tools/cmd dir, and later add link in Makefile. But I honestly don't like glide a little. I think manually updating the vendor very occasionally is OK.

@xiekeyang
Copy link
Owner Author

LGTM

@xiekeyang xiekeyang merged commit 69c0773 into master Sep 21, 2017
Signed-off-by: xiekeyang <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants