docs: remove references to vgo #206

zombiezen · 2018-07-18T16:33:10Z

We should be instructing users to run stable things, and more adventurous folks can adapt to use Go modules. I just ran the test suite locally from a go get and it compiled and ran fine.

cflewis · 2018-07-18T19:34:49Z

I don't like this change unless we are also running it in Travis. We have certainly had issues in the past where updated modules that we didn't have locally broke the build. Remember when I was steadfastly not using vgo in order to dogfood that and keep things up to date, and then kinda gave up?

So unless this PR also modifies the Travis CI build to use go get instead of vgo, I think we should drop it as there's too much possibility for things to go wrong.

Isn't vgo getting rolled into go 1.11 anyway? Can't we just hold for a little longer and then do this?

zombiezen · 2018-07-18T19:52:06Z

I'm taking precedence from this post by @rsc, specifically:

I certainly do appreciate that everyone on this list is excited to use modules as much as possible, myself included. We all just need to remember that not all our users are like us. In fact most aren't. They just want to get work done and adopt new potentially-breaking features on their schedule, not ours.

It seems better to phrase the installation instructions in terms of stable features.

We could try to do a mixed go and vgo build (I have this set up for gg, for instance), but I worry that we wouldn't be able to figure out what versions go get grabbed.

cflewis · 2018-07-18T22:05:34Z

I'm not saying we shouldn't do it, just that if we do do this it has to have the Travis build modified. A hybrid build is fine for me. We can't put ourselves in a position where we're instructing customers to something different to what we are doing locally, but we aren't automatically testing that workflow.

enocom

Navigating the transition to Go modules is a little tricky. We want to provide a good experience for users who haven't adopted modules while likewise making modules easy to use with Go Cloud. For now, we're probably OK with instructions to use vgo, but we'll need to update those to point to the beta release of Go 1.11 when it becomes available, I think.

enocom · 2018-07-18T23:07:50Z

CONTRIBUTING.md

@@ -48,7 +48,7 @@ While we figure out how to properly generate replay files for contributors, writ

 ## Making a pull request
 * Follow the normal [pull request flow](https://help.github.com/articles/creating-a-pull-request/)
-* Build your changes using `vgo build .`, _do not use `go`_. Go Cloud only supports `vgo` in order to ensure [reproducible builds](https://research.swtch.com/vgo-repro).
+* Build your changes using `vgo build .`, _do not use `go`_. Go Cloud CI uses `vgo` in order to ensure [reproducible builds](https://research.swtch.com/vgo-repro).


With vgo merged into Go's develop branch, I think "Go modules" is the preferred term. Perhaps we should call out support for modules with Go 1.11 beta 2 (?) instead of vgo.

rsc · 2018-07-19T00:15:24Z

I think it would be OK to say that you're maintaining your dependencies with the Go modules as will be shipping in Go 1.11, and that if people need to modify the set of dependencies then they should use that. We're looking at beta2 with module support going out tomorrow.

If you really need to support pre-Go1.11 users the thing to do would be to run 'go mod -vendor' and check in the vendor directory. But that's just going to blow up the repo for a temporary need.

zombiezen · 2018-07-19T15:23:05Z

As another data point, @ijt ran into #208 because Wire does not handle loading Go modules (#78). I think it's important to keep people on GOPATH for a bit until we can address the tooling issue. If dependencies make breaking changes, we can update the go.mod file as needed.

cflewis · 2018-07-19T15:49:46Z

I don't have any strong feelings either way on whether to wait or change. @ijt modified some docs for #208 FWIW.

We should be instructing users to run stable things, and more adventurous folks can adapt to use Go modules. I just ran the test suite locally from a `go get` and it compiled and ran fine. Updates #78 Updates #208

zombiezen

PTAL

zombiezen · 2018-07-19T16:12:00Z

CONTRIBUTING.md

@@ -48,7 +48,7 @@ While we figure out how to properly generate replay files for contributors, writ

 ## Making a pull request
 * Follow the normal [pull request flow](https://help.github.com/articles/creating-a-pull-request/)
-* Build your changes using `vgo build .`, _do not use `go`_. Go Cloud only supports `vgo` in order to ensure [reproducible builds](https://research.swtch.com/vgo-repro).
+* Build your changes using `vgo build .`, _do not use `go`_. Go Cloud CI uses `vgo` in order to ensure [reproducible builds](https://research.swtch.com/vgo-repro).


cflewis · 2018-07-19T18:08:13Z

LGTM. Please wait for @ijt approval.

ijt · 2018-07-19T18:17:17Z

LGTM.

ijt · 2018-07-19T18:19:04Z

README.md

-$ cd go-cloud
-$ vgo install ./wire/cmd/gowire
+go get github.com/google/go-cloud
+go install github.com/google/go-cloud/wire/cmd/gowire


Why not go get for this also? It works as far as I can tell.

We should be instructing users to run stable things, and more adventurous folks can adapt to use Go modules. I just ran the test suite locally from a `go get` and it compiled and ran fine. Updates google/go-cloud#78 Updates google/go-cloud#208

zombiezen requested review from cflewis and enocom July 18, 2018 16:33

zombiezen added this to the Sprint 14 milestone Jul 18, 2018

enocom reviewed Jul 18, 2018

View reviewed changes

zombiezen mentioned this pull request Jul 19, 2018

In README, install go-cloud to $GOPATH. #209

Closed

zombiezen requested a review from ijt July 19, 2018 16:07

zombiezen added 2 commits July 19, 2018 09:11

docs: remove references to vgo

94b21d9

We should be instructing users to run stable things, and more adventurous folks can adapt to use Go modules. I just ran the test suite locally from a `go get` and it compiled and ran fine. Updates #78 Updates #208

Use term 'modules' in CONTRIBUTING

2b3497a

zombiezen commented Jul 19, 2018

View reviewed changes

cflewis approved these changes Jul 19, 2018

View reviewed changes

ijt approved these changes Jul 19, 2018

View reviewed changes

zombiezen added 2 commits July 19, 2018 11:21

Use go get for both commands

2d3ce42

Merge branch 'master' into no-vgo

be86ef0

zombiezen merged commit 6601c19 into google:master Jul 19, 2018

zombiezen deleted the no-vgo branch July 19, 2018 20:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs: remove references to vgo #206

docs: remove references to vgo #206

zombiezen commented Jul 18, 2018 •

edited

Loading

cflewis commented Jul 18, 2018

zombiezen commented Jul 18, 2018

cflewis commented Jul 18, 2018

enocom left a comment

enocom Jul 18, 2018

zombiezen Jul 19, 2018

rsc commented Jul 19, 2018

zombiezen commented Jul 19, 2018

cflewis commented Jul 19, 2018

zombiezen left a comment

zombiezen Jul 19, 2018

cflewis commented Jul 19, 2018

ijt commented Jul 19, 2018

ijt Jul 19, 2018

zombiezen Jul 19, 2018

docs: remove references to vgo #206

docs: remove references to vgo #206

Conversation

zombiezen commented Jul 18, 2018 • edited Loading

cflewis commented Jul 18, 2018

zombiezen commented Jul 18, 2018

cflewis commented Jul 18, 2018

enocom left a comment

Choose a reason for hiding this comment

enocom Jul 18, 2018

Choose a reason for hiding this comment

zombiezen Jul 19, 2018

Choose a reason for hiding this comment

rsc commented Jul 19, 2018

zombiezen commented Jul 19, 2018

cflewis commented Jul 19, 2018

zombiezen left a comment

Choose a reason for hiding this comment

zombiezen Jul 19, 2018

Choose a reason for hiding this comment

cflewis commented Jul 19, 2018

ijt commented Jul 19, 2018

ijt Jul 19, 2018

Choose a reason for hiding this comment

zombiezen Jul 19, 2018

Choose a reason for hiding this comment

zombiezen commented Jul 18, 2018 •

edited

Loading