Add vendoring with govendor #2461

Merged
merged 1 commit into from Sep 18, 2016

Projects

None yet

3 participants

@moorereason
Collaborator
moorereason commented Sep 16, 2016 edited

Doesn't check in vendored packages. Rewrites Makefile to use govendor helpers.

@moorereason moorereason Add vendoring with govendor
bd4be08
@moorereason moorereason changed the title from WIP: Add vendoring to Add vendoring with govendor Sep 16, 2016
@bep
Collaborator
bep commented Sep 17, 2016

The vendor.json file; is that a standard format?

@moorereason
Collaborator

There is no standard format. That's govendor's format. The only standard is that the Go tools look for vendored packages in the vendor/ directory.

@moorereason moorereason added this to the v0.17 milestone Sep 18, 2016
@moorereason
Collaborator

Tagged for 0.17. FIxes #2119.

@bep bep merged commit 6e692f2 into spf13:master Sep 18, 2016

4 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/wercker Wercker build passed
Details
licence/cla Contributor License Agreement is signed.
Details
@bep
Collaborator
bep commented Sep 18, 2016

I merged this as it does solve the problem ... but we should do an evaluation of the vendoring options for 0.18.

@nathany
Contributor
nathany commented Oct 18, 2016

When I read the release notes for 0.17, I just expected the code under vendor/ to be checked in.

Should make govendor be mentioned in the README?

@@ -34,39 +32,45 @@ docker:
docker cp hugo-build:/go/bin/hugo .
docker rm hugo-build
+govendor:
+ go get -u github.com/kardianos/govendor
+ go install github.com/kardianos/govendor
@nathany
nathany Oct 18, 2016 Contributor

FYI

"Get downloads the packages named by the import paths, along with their
dependencies. It then installs the named packages, like 'go install'." - go get -h

@nathany
nathany Oct 18, 2016 Contributor

Which is to say the go install line is superfluous.

It still appears here atm: https://github.com/spf13/hugo/blob/master/Makefile#L37

@moorereason
moorereason Oct 18, 2016 Collaborator

You're right. I was looking at this on my phone and misunderstood where your comment was in the Makefile. The go install does indeed need to be removed.

@moorereason
Collaborator

The Makefile has already been fixed in master.

And yes, we may need to update the README.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment