Skip to content
This repository has been archived by the owner on Mar 6, 2020. It is now read-only.

Upstream gvt restore patches: delete old deps and parallel downloads #547

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

FiloSottile
Copy link
Contributor

gvt had a command called rebuild, which looked exactly like restore (because I'm an idiot and didn't remember removing restore when doing the first port).

I renamed it restore, and this PR upstreams the two main changes that were applied to gvt rebuild:

@@ -11,23 +14,35 @@ import (
"github.com/constabulary/gb/vendor"
)

var (
rbInsecure bool // Allow the use of insecure protocols
Copy link
Contributor

Choose a reason for hiding this comment

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

I cannot see where this is being set

@davecheney
Copy link
Contributor

Look, this is going to be an awkward conversation, but I don't want to take any patches for gb vendor restore. It was a mistake to accept that feature in the first place because it confuses the previous story that gb handles dependencies through vendoring into $PROJECT/vendor/src.

If you don't want to, or cannot vendor your dependency's source then I offer proposal #536 as an alternative.

Thanks for your understanding.

@FiloSottile
Copy link
Contributor Author

I fully expected that reply. I'm equally unhappy about gvt restore. However, these are patches that I had accepted a long time ago in gvt, and I'm really trying to keep the core of gvt in sync with gb-vendor (for my own sanity).

I believe these are pretty technical changes, nothing behavioral, so I gave upstreaming a try. I'd still be happier if this was accepted, after which I'd stop taking patches on gvt restore, too. But I'll understand if you close it, and I'll just keep the commits around and apply them to gb master if I need it when syncing.

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.

2 participants