check: gometalinter showing errors

[grahamwhaley]

Running a make check shows errors (below) for me from gometalinter et. al.

I nuked any currently installed gometalinters, 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.

[sboeuf]

@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.

[grahamwhaley]

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 the go-lint.sh ?
• Q. Do we fix our gometalinter version to a fixed version (v2.0.5 being the latest), to avoid 'lint rot' ??

[jodh-intel]

I'll fix the network.go issues on #648...

[jodh-intel]

PR is #649 for the network fixes.