Migrate to go mod and golangci-lint #51
Conversation
|
@chrishajas @khuddlefish @pf-qiu @fanfuxiaoran @dsharp-pivotal
|
|
We don't use master branch so shouldn't be a problem for us. |
Maybe separate issue, just curious why needs to maintain your code in another branch? |
| go get golang.org/x/tools/cmd/goimports | ||
| go get github.com/onsi/ginkgo/ginkgo | ||
| go mod vendor | ||
| go mod tidy |
There was a problem hiding this comment.
This also seems unnecessary. It could cause unexpected diffs.
There was a problem hiding this comment.
Hi, @dsharp-pivotal
From the best practise, It's always recommended to run go mod tidy after go install or go build.
Here's some explanation from golang repo:
golang/go#27633
The reason is that go mod tidy is agnostic of GOOS/GOARCH or build tags; go build is lazy and only does the minimal amount of work required for the current GOOS/GOARCH and build tags.
cc @bcmills - I wonder whether we should update the documentation for go mod tidy to include some note about this operation being agnostic of GOOS/GOARCH and build tags:
...
Because one reading of the documentation is that go build and go mod tidy should leave go.mod in the same state.
There was a problem hiding this comment.
I kept the go mod tidy statement according to above explanation from yangru
| @cd vendor/golang.org/x/tools/cmd/goimports; go install . | ||
| @cd vendor/github.com/onsi/ginkgo/ginkgo; go install . | ||
| go get golang.org/x/tools/cmd/goimports | ||
| go get github.com/onsi/ginkgo/ginkgo |
There was a problem hiding this comment.
Should these be go install?
There was a problem hiding this comment.
go get github.com/onsi/ginkgo/ginkgo will install the binary executable into $GOPATH/bin/ so don't need to run go install.
The same with goimports.
Makefile
Outdated
| @@ -12,8 +12,7 @@ GOFLAGS := | |||
| .PHONY : coverage | |||
|
|
|||
| dependencies : | |||
| go get github.com/alecthomas/gometalinter | |||
| gometalinter --install | |||
| curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.21.0 | |||
There was a problem hiding this comment.
FWIW, we install with go install -i github.com/golangci/golangci-lint/cmd/golangci-lint
There was a problem hiding this comment.
According to the readme of golangci-lint https://github.com/golangci/golangci-lint#binary, it suggests use curl to install.
It says Please, do not install golangci-lint by go get:
1. go.mod replacement directive doesn't apply. It means you will be using patched version of golangci-lint.
2. it's much slower than binary installation
3. its stability depends on your Go version (e.g. on this compiler Go <= 1.12 bug).
4. it's not guaranteed to work: e.g. we've encountered a lot of issues with Go modules hashes.
5. it allows installation from master branch which can't be considered stable.
I think go install is the same with with go get, it will pollute the go.mod/go.sum files.
Deprecated
Migrated to