Add support for godep #77
Conversation
- Install godep in Dockerfile - Add update-deps make command - Enable vendor experiment
|
I know you're coming back to this but we definitely don't want to have the vendored dependencies committed in this repo, so we should add that to the |
|
@tgross I can ignore that folder; however, the In any case, godep restore would put all of the dependencies in the |
|
Alternatively, we could look into govendor instead |
Only if we're using git subtrees or submodules. Vendoring the dependencies to the appropriate file system location is an orthogonal concern to the source control for those dependencies. Embedding them within our code obscures provenance and also fills our project history with someone else's project. |
|
amended commit - TravisCI build failed due to a curl error - no clear way to ask it to try again. |
I agree - I hate vendoring in general. It seems like the way the go team wants you to treat I've excluded it from this PR; However, in doing so we are deviating from the intended use. |
| mkdir -p "${ROOT}/src/github.com/coreos" | ||
| git clone https://github.com/coreos/etcd.git ${ROOT}/src/github.com/coreos/etcd | ||
| cd ${ROOT}/src/github.com/coreos/etcd && git checkout ${ETCD_REF} | ||
| update-deps: restore |
There was a problem hiding this comment.
Am I misunderstanding this or does this update all our dependencies to the current tip? Can we put a giant "OMG don't do this" warning on the top of this?
There was a problem hiding this comment.
Could we maybe get a make restore at the top-level make too?
There was a problem hiding this comment.
I could remove this completely, we could take manual steps to upgrade the library ourselves.
|
I'm seeing this in the Travis output:
I think we want to make sure that directory isn't there. |
Ok, but they're profoundly wrong and I'm ok with taking a stance on that. |
| ${CDWD} && go vet | ||
| ${CDWD} && go fmt | ||
| golint ${ROOT}/src/containerbuddy | ||
| ${CDWD} && go vet ${NO_VENDOR} |
There was a problem hiding this comment.
Unfortunate artifact of how /vendor folder works. ./... path expansion cannot be used because it would recurse into the vendor folder. you have to explicitly exclude it
|
Ok, thanks for slogging thru this @justenwalker. LGTM and I'm going to merge it in and then cut a hopefully-finally RC. |
For #76