-
Notifications
You must be signed in to change notification settings - Fork 452
Respect the GO15VENDOREXPERIMENT env variable #244
Conversation
The GO15VENDOREXPERIMENT env variable is how the vendor experiment is enabled and we should respect that and use it when the user has it set.
I agree. What else do you think it would need? |
var sep = "/Godeps/_workspace/src/" | ||
var VendorExperiment = os.Getenv("GO15VENDOREXPERIMENT") == "1" | ||
|
||
func init() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest the signature defaultSep(experiment bool) string
for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. I likened it to flag handling, which is done elsewhere in init functions.
Are you suggesting that init should call defaultSep(VendorExperiment)
to set sep or that everywhere sep is currently used it should be replaced with defaultSep(VendorExperiment)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm suggesting the first, e.g. var sep = defaultSep(VendorExperiment)
.
Flags are defined in init functions only because they have to to avoid a cycle. (Try changing it to var saveR = cmdSave.Flag.Bool(...)
to see what happens.)
-Don't use an init function -Added/updated some docs
@kr Updated with suggestions. Re: What more. TBH I think this is enough for now. It's usable for me w/o changes to my workflow (modulo golang/go#11659 anyway). |
@kr just to be clear I don't think that golang issue should block. Just pointing it out. |
LGTM |
@kr I also added docs and fixed an issue, so if you could take another look ++. |
What do you think about putting Godeps.json inside vendor? It would mean more work for other tools that read that file (e.g. the Heroku buildpack) but it would produce less clutter in a project's root directory. Either way, LGTM |
@kr I think I'd rather leave the file where it is. For now at least. |
Looks like this was closed in fe7138c. |
@kr Yeah, sorry I got distracted after the merge with other work. |
Does this mean that |
@pierrre Not 100%, but essentially yes. |
The GO15VENDOREXPERIMENT env variable is how the vendor experiment is
enabled and we should respect that and use it when the user has it set.
@kr ... I think this is something (with maybe a little more work) we should actually incorporate sooner rather than later. Thoughts?