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

Opt for more safety on whether to regenerate vendor #1310

Closed
wants to merge 1 commit into from

Conversation

sdboyer
Copy link
Member

@sdboyer sdboyer commented Oct 25, 2017

What does this do / why do we need it?

We were guessing that, if the Gopkg.lock hadn't changed, that vendor
also didn't need to change. However, that missed cases where manual
modifications had been made to vendor. It's not terribly common, but it
still violates the sync model to get this wrong.

/cc @ibrasho this is ensure, so i'll leave this for you to decide on and merge

What should your reviewer look out for in this PR?

Do you need help or clarification on anything?

Which issue(s) does this PR fix?

fixes #1276

We were guessing that, if the Gopkg.lock hadn't changed, that vendor
also didn't need to change. However, that missed cases where manual
modifications had been made to vendor. It's not terribly common, but it
still violates the sync model to get this wrong.
@sdboyer sdboyer added this to the v0.3.3 milestone Oct 25, 2017
@sdboyer sdboyer requested a review from ibrasho as a code owner October 25, 2017 01:51
@sdboyer
Copy link
Member Author

sdboyer commented Oct 25, 2017

...i'm actually kinda on the fence about this. is it just enough to ask people to run dep ensure -vendor-only in this case? seems like maybe it should be, and we can reject this PR.

🤔

@sdboyer
Copy link
Member Author

sdboyer commented Oct 30, 2017

yeah, i think we can safely get away with not doing this for now.

@sdboyer sdboyer closed this Oct 30, 2017
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.

dep ensure -update doesn't realize when a vendored directory has been removed
2 participants