linting: added errcheck, gocritic, goimports, golint, govet & megacheck

[lopezator]

added linters using golangci. fix linted errors.

references #47

[lopezator]

Just a first pass.

Needs revision, maybe adjustements, and a explanation of how to install golangci-lint.

Maybe add instructions to README?

Integration of sanity-check target with CI?

CC @F21

[F21]

@lopezator Thank you so much for working on this!

Needs revision, maybe adjustements, and a explanation of how to install golangci-lint.

Maybe add instructions to README?

Definitely a good idea to add instructions for running golangci-lint to the readme. I think we can add a section before the License section.

Integration of sanity-check target with CI?

For this one, you'd need to edit wercker.yml. I'd suggest adding another script step before the go test step to install golangci-lint and run the target in the makefile. I am not sure if make is included in the Go alpine docker image by default, but if it isn't, it will need to be installed in the set up build tools step.

[lopezator]

PTAL @F21

Feel free to suggest/change anything. English is not my native language and the description in the README.md file for example, might not be clear enough.

I left the Makefile although it is calling only golangci-lint run in case we need to add more checks to the sanity-check target, like go mod verify or whatever without having the need to change the wercker.yml file.

Cannot make the build pass, looks something related to go modules, not sure.

[F21]

@lopezator Thanks again for working on this! There are a few minor changes I'd like for the README.md file, but let's see if we can fix the go modules error first.

I did a quick search and it appears that golangci-lint supports go modules: golangci/golangci-lint#205

I believe what we need to do is add golangci-lint to our go.mod. See https://github.com/golang/go/wiki/Modules#how-can-i-track-tool-dependencies-for-a-module

In essence, you'll need to create a tools.go file and import golangci-lint with an underscore and add a build constraint // +build tools. Then you need to go get golangci-lint to add it to go.mod.

In the wercker.yml file, rather than installing the golangci-lint binary, we use go install to install it. This should install the correct version of golangci-lint as specified in our go.mod file.

I haven't had the chance to give this a shot, but I think it should work.

[lopezator]

after messing around with this wercker thing for a while it seems I've been able to pass the checks. PTAL @F21 let's see if we can polish this :)

[F21]

@lopezator Thanks for your work on this! The problem with using base-path is that the project is loaded into the GOPATH and GO111MODULE would be set to auto. In general, I prefer to keep the code out of the GOPATH when using go modules.

I have pushed a commit that does the following:

1. Pin the golangci-lint version in go.mod and added it to tools.go. This way, running go install github.com/golangci/golangci-lint/cmd/golangci-lint will always install the same version.
2. Regenerated go.sum because it was generated using an older version of Go 1.11 and the checksum algorithm changed between patch releases (causing mismatch errors).
3. Made some adjustments to the README.

The commit is on the golint branch in this repo: 44263f4

The CI for my fork also passes except for code climate (due to some missing config): https://app.wercker.com/F21/migration/runs/build/5c37cb980f540f0025857fb8?step=5c37cbc2cfa0fc000779b757

Can you squash my commit with yours into 1 commit (you should be set as the author of the commit)? I think we should be ready to merge 👍

[lopezator]

I've cherry-picked that commit and squashed it into mine, thanks for the help @F21

PTAL again 🙂

[F21]

Looks like I messed up a bit here. Can you change it to We automatically run?

[lopezator]

sure.

[F21]

Many thanks for pulling in the changes. I messed up a bit with the README, do you mind making the change and squashing?

[F21]

Thanks @lopezator !