Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add vendoring with govendor #2461

Merged
merged 1 commit into from Sep 18, 2016
Merged

Add vendoring with govendor #2461

merged 1 commit into from Sep 18, 2016

Conversation

moorereason
Copy link
Contributor

@moorereason moorereason commented Sep 16, 2016

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

@moorereason moorereason changed the title WIP: Add vendoring Add vendoring with govendor Sep 16, 2016
@bep
Copy link
Member

bep commented Sep 17, 2016

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

@moorereason
Copy link
Contributor Author

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
Copy link
Contributor Author

Tagged for 0.17. FIxes #2119.

@bep bep merged commit 6e692f2 into gohugoio:master Sep 18, 2016
@bep
Copy link
Member

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
Copy link
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which is to say the go install line is superfluous.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Contributor Author

The Makefile has already been fixed in master.

And yes, we may need to update the README.

@moorereason moorereason deleted the govendor branch March 1, 2017 15:38
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants