Set GO111MODULE to auto in golint script– #1743 #1744
Conversation
As mentioned in gomods#1743 the Go modules environment flag is set to `off` in the script which appears to cause a warning message for each module of the codebase that it is "not in GOROOT". Set to `auto` as this allows the same build to be run the original way should someone choose to delete the `go.mod` file from the project root.
scripts/check_golint.sh
Outdated
| @@ -4,4 +4,4 @@ | |||
| # Run the linter on everything except generated code | |||
| set -euo pipefail | |||
|
|
|||
| golint -set_exit_status $(GO111MODULE=off go list ./... | grep -v '/mocks') | |||
| golint -set_exit_status $(GO111MODULE=auto go list ./... | grep -v '/mocks') | |||
There was a problem hiding this comment.
Thanks for this change. I believe we have now started to build for go1.17.
Go1.17 will ignore this flag: https://go.dev/blog/go116-module-changes.
I believe: #815 was an explicit choice though to make the CI faster
I am not sure if this will work anymore. Maybe we should just remove it: cc @marwan-at-work ?
There was a problem hiding this comment.
Yes I think it's worth removing. Also, do we even have /mock? I wonder if we can do golinst ./... or golint go list ./...
There was a problem hiding this comment.
I'll do it right and raise another PR!
There was a problem hiding this comment.
I checked golint ./... vs. golint $(go list ./...)... I am struggling to prove it definitively prove that the former is linting all the files covered by the latter. The go list subshell command version takes noticeably longer, but that's not to say, necessarily, that the golint command isn't examining all the correct files. Any input on that much appreciated.
There was a problem hiding this comment.
Thank you for doing it right. I just noticed that even golint is deprecated.
https://github.com/golang/lint
Would you be able to update the code to use staticcheck and go vet?
We already use govet
Line 32 in fd54bab
I would probably just start with govet, open an issue and then do a separate PR for staticcheck. I hope this helps.
Go 1.17 ignores `GO111MODULE` and there are no directories in the project root called `mock`.
`golint` is deprecated (and frozen) replace with the current `go vet`. This reported one issue on `main` branch: ```pkg/stash/with_etcd.go:33:28: loop variable ep captured by func literal``` Fixed loop variable capture with extraction to parameterised anonymous function passed loop variable and passed in to `errgroup.Go` call.
|
Thanks @manugupt1 I have made the change to |
|
Thanks for making this change. Also congrats on your first PR 🎉 |
|
@rbrtl this is awesome - thank you! |
As mentioned in #1743 the Go modules environment flag is set to
offin the script which appears to cause a warning message for each module
of the codebase that it is "not in GOROOT".
Set to
autoas this allows the same build to be run the original wayshould someone choose to delete the
go.modfile from the project root.What is the problem I am trying to address?
Golint warning for each submodule of Athens "package not in GOROOT" when running Make target
verifyHow is the fix applied?
Set
GO111MODULE=autoin./scripts/check_golint.shWhat GitHub issue(s) does this PR fix or close?
Fixes #1743