check: gometalinter showing errors
grahamwhaley opened this issue · 4 comments
Running a make check
shows errors (below) for me from gometalinter
et. al.
I nuked any currently installed gometalinter
s, and installed the latest version:
$ go get -u github.com/alecthomas/gometalinter
and asked it to update all the linters (even though the help says DEPRECATED, but the docs say to do this):
$ gometalinter --install
and then ran a check:
$ make check
I then get >2k lines of output. Here is the head of that:
api.go:308::warning: declaration of "err" shadows declaration at api.go:290 (vetshadow)
container.go:419::warning: declaration of "err" shadows declaration at container.go:414 (vetshadow)
container.go:428::warning: declaration of "err" shadows declaration at container.go:414 (vetshadow)
container.go:435::warning: declaration of "err" shadows declaration at container.go:414 (vetshadow)
container.go:443::warning: declaration of "err" shadows declaration at container.go:414 (vetshadow)
container.go:526::warning: declaration of "err" shadows declaration at container.go:523 (vetshadow)
hyperstart_agent.go:346::warning: declaration of "err" shadows declaration at hyperstart_agent.go:338 (vetshadow)
hyperstart_agent.go:352::warning: declaration of "err" shadows declaration at hyperstart_agent.go:338 (vetshadow)
hyperstart_agent.go:419::warning: declaration of "err" shadows declaration at hyperstart_agent.go:402 (vetshadow)
hyperstart_agent.go:425::warning: declaration of "err" shadows declaration at hyperstart_agent.go:402 (vetshadow)
hyperstart_agent.go:435::warning: declaration of "err" shadows declaration at hyperstart_agent.go:402 (vetshadow)
kata_agent.go:133::warning: declaration of "err" shadows declaration at kata_agent.go:130 (vetshadow)
kata_agent.go:391::warning: declaration of "proxyParams" shadows declaration at proxy.go:36 (vetshadow)
Do we have a requirement for gometalinter
version? - and if so, I'll document in the dev docs and fix https://github.com/containers/virtcontainers/blob/master/.ci/go-lint.sh#L21 to use the appropriately named gometalinter.vX
Oh, probably makes no difference in this case, but I'm on go v1.8.3 for reference.
@grahamwhaley if you go with the following command:
sudo -E PATH=$PATH CI=true bash -c "make check"
What do you get ?
The CI
flag allows to be more permissive here.
Yep, that is much better. Note, we don't really need the sudo
or the bash
though:
$ CI=true make check
bash .ci/go-lint.sh
network.go:1268:16:warning: ineffectual assignment to err (ineffassign)
network.go:1270:16:warning: ineffectual assignment to err (ineffassign)
qemu.go:595:15:warning: ineffectual assignment to err (ineffassign)
Makefile:52: recipe for target 'check-go-static' failed
make: *** [check-go-static] Error 1
I had a look at those warnings. The qemu.go one is pretty obvious, but the network ones not quite so to my eyes:
https://github.com/containers/virtcontainers/blob/master/network.go#L1268
I think those lines are a combination of mods by @sboeuf @egernst @jcvenegas
If anybody would care to cast their golang eyes over, and maybe PR some ineffassign fixups?
Now, what to do with make check
.
- Q. Do we set
CI=true
always in the Makefile or thego-lint.sh
? - Q. Do we fix our
gometalinter
version to a fixed version (v2.0.5 being the latest), to avoid 'lint rot' ??
I'll fix the network.go
issues on #648...
PR is #649 for the network fixes.