Switch to Go modules

[dennwc]

Switch import paths from gopkg.in to GitHub and switch Dep to Go modules.

This also bumps the client version to v4.

[dennwc]

Waiting for someone to enable CIs :)

[bzz]

LGTM

And I guess at least 2 pages at documentation need to have paths updated as well?

[dennwc]

@bzz Definitely. Also, still waiting for AppVeyor CI pass.

[bzz]

JFYI go test all on current master does not seem to pass.

Sorry for not noticing that earlier. Similar failures were rised as relevant in module migration in enry so posting it here:

# github.com/pkg/errors
vet: ../../../go/pkg/mod/github.com/pkg/errors@v0.8.0/bench_test.go:26:6: toperr declared but not used
FAIL	github.com/pkg/errors [build failed]

[dennwc]

@bzz go test all runs test not only for the current project but for all other packages in GOPATH (or required by the module). Thus, this failure is not related to Go client - it's a test failure in github.com/pkg/errors. All we can do is bump the dependency version when they fix it.

[bzz]

I know, @dennwc and thought the same, it's just that a different opinion was expressed by @creachadair at src-d/enry#219 (comment) so I'm just sharing it here.

Would be good to treat those uniformly somehow, given that there is no conceptual difference between these two.

[creachadair]

I know, @dennwc and thought the same, it's just that a different opinion was expressed by @creachadair at src-d/enry#219 (comment) so I'm just sharing it here.

Would be good to treat those uniformly somehow, given that there is no conceptual difference between these two.

Well, we should probably not be using go test all in our CI, which I think would simplify both cases.

[bzz]

Agree! But I'm a bit confused - I'm not aware of anyone using go test all on CI so far.

[creachadair]

Agree! But I'm a bit confused - I'm not aware of anyone using go test all on CI so far.

Sorry, based on #118 (comment) I thought maybe you were saying our failures were due to that. If go test ./... from the module root passes, I think we should be OK.

[bzz]

I'm sorry for digging this up - but @dennwc do you think we still need this for some reason, given GO111MODULE=on?

[dennwc]

We can try disabling it. The only reason to do this is to allow CI to pass in forks.

[bzz]

Yes, thank you for confirming - that is what I thought as well.

On the second thought - would not CI on forks still work, even without it, as GO111MODULE=on ?

[dennwc]

I think it should be able to infer the import path from go.mod anyway, so you are right, it should work. Let me try to open a PR with it, we'll see if it fails or not on my fork.

[bzz]

😆 thank you for brave experiments!

[dennwc]

#120

[bzz]

BTW, do you think this is necessary for correctness, or is it just an optimisation to warm up some FS cache between jobs?

[dennwc]

Yes, it's only to split the dependency download process and the test process. It will work if we remove it, but it will download everything during go test.