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

[Go] Update ignore patterns. #2266

Merged
merged 3 commits into from Jan 31, 2017
Merged

[Go] Update ignore patterns. #2266

merged 3 commits into from Jan 31, 2017

Conversation

@AlekSi
Copy link
Contributor

@AlekSi AlekSi commented Jan 30, 2017

Remove old patterns for legacy Go versions (pre 1.0, when Makefiles were used instead of go command).

It hurts me to see those remnants over and over again.

Remove vendor/.

To quote https://blog.gopheracademy.com/advent-2015/vendor-folder/:

Advice on how to use the vendor folder is varied. Some will shutter at the thought of including dependencies in the project repository. Others hold it is unthinkable to not include the dependencies in the project repository. Some will just want to include a manifest or lock file and fetch the dependencies before building.

Go doesn't have widely used gem and rubygems.org equivalent yet. Instead, several third-party tools are used, which fetch dependencies from Github and other places, and lock versions to particular commit hashes. go tool is then used to build programs. go is aware of vendor/ directory, but knows nothing about lock files, commit hashes, etc. If vendor/ is not included in the repository, then go get will download sources – but it will be the latest commit for each import path (more or less is equivalent of repository URL), not specified commits from lock file. In you are lucky, and all package authors do care about backward compatibility (but that's not true in general – many packages are broken regularly), it may work. If you are less lucky, you will get a broken build. In the worst case, your code will explode in runtime, not during compilation. Therefore, I'm strongly for inclusion of vendor/. I think the majority of the community thinks the same way.

Even people who disagree with this opinion would agree that omitting vendor/ should be opt-in, not opt-out.

Related pull requests:
#2198 (closed as duplicate).
#2183 (closed due to conflicts).

@AlekSi
Copy link
Contributor Author

@AlekSi AlekSi commented Jan 30, 2017

/cc @shiftkey

@shiftkey
Copy link
Member

@shiftkey shiftkey commented Jan 31, 2017

@AlekSi given the significant number of 👍 here in the first 24 hours, and how much it cleans things up, I'm happy to take this in. Thanks!

@shiftkey shiftkey merged commit bcb05d9 into github:master Jan 31, 2017
@AlexandreRoba

This comment has been minimized.

Copy link

@AlexandreRoba AlexandreRoba commented on 0dae103 Feb 6, 2017

Hi there, I have a question. Why removing the vendor folder? Any reason behind that?

This comment has been minimized.

Copy link
Contributor Author

@AlekSi AlekSi replied Feb 6, 2017

See #2266

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants