Skip to content
This repository was archived by the owner on Jul 22, 2020. It is now read-only.

Stop committing vendor dir to git#155

Merged
prymitive merged 4 commits intomasterfrom
cache-vendor
Aug 8, 2017
Merged

Stop committing vendor dir to git#155
prymitive merged 4 commits intomasterfrom
cache-vendor

Conversation

@prymitive
Copy link
Contributor

vendor is committed to git mostly to speed up CI builds, but since the CI can cache this dir we can drop it and just enable cache

Copy link

@terinjokes terinjokes left a comment

Choose a reason for hiding this comment

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

I have 24 GB of RAM, and reviewing this PR took all of it.

@prymitive prymitive merged commit b0b7e94 into master Aug 8, 2017
@prymitive prymitive deleted the cache-vendor branch August 8, 2017 18:14
@mattbostock
Copy link
Contributor

This change means that if one of the dependencies is no longer available to download, Unsee builds will break until you fix Unsee to either update the package path or rework the code to use another library.

Since Go has no central repository for packages like RubyGems (GitHub is probably the closest equivalent), it's usually recommended to vendor dependencies. If we're happy to risk new builds breaking at any time, this change is fine.

@prymitive
Copy link
Contributor Author

We have docker images already build at that point.
If any upstream repo is gone we need to address it quickly rather than keep an maintained copy of it and only notice when we try to update it.

@mattbostock
Copy link
Contributor

@prymitive: The occasion where that approach falls short is when you need to cut a new release in a hurry, for example to fix a security issue. Unsee is intended as an internal tool, which mitigates that problem to some extent.

I think we should either document our expectations around this (i.e. Unsee is for internal use only in 'trusted' environments), or vendor our dependencies so we are in a position to cut 'hotfix' releases if needs be. My preference is for the latter.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants