Add linter configuration

[epk]

Description

Adding a linter configuration file for deterministic linting

[epk]

Sorry about the excessive review pings

[shlomi-noach]

Thanks so much for working on this, please see questions inline

[shlomi-noach]

How did you pick this specific list of flags? (To clarify, I'm not sure what the list should be)

[epk]

I ran golangci-lint linters in a directory without a .golangci.yml present

 ~  golangci-lint linters
Enabled by your configuration linters:
deadcode: Finds unused code [fast: true, auto-fix: false]
errcheck: Errcheck is a program for checking for unchecked errors in go programs. These unchecked errors can be critical bugs in some cases [fast: true, auto-fix: false]
gosimple (megacheck): Linter for Go source code that specializes in simplifying a code [fast: true, auto-fix: false]
govet (vet, vetshadow): Vet examines Go source code and reports suspicious constructs, such as Printf calls whose arguments do not align with the format string [fast: true, auto-fix: false]
ineffassign: Detects when assignments to existing variables are not used [fast: true, auto-fix: false]
staticcheck (megacheck): Staticcheck is a go vet on steroids, applying a ton of static analysis checks [fast: true, auto-fix: false]
structcheck: Finds unused struct fields [fast: true, auto-fix: false]
typecheck: Like the front-end of a Go compiler, parses and type-checks Go code [fast: true, auto-fix: false]
unused (megacheck): Checks Go code for unused constants, variables, functions and types [fast: false, auto-fix: false]
varcheck: Finds unused global variables and constants [fast: true, auto-fix: false]

I removed errcheck from the list as the previous CLI invocations had it disabled

[shlomi-noach]

+1 for fixed version

[shlomi-noach]

Since errcheck is removed, should we expect new linter errors on existing code?

[ajm188]

errcheck is implicitly removed by the combination of disable-all: true and errcheck not being in the enable list in the config file

[ajm188]

Also, I wanted to ask: for the commit hook specifically, can we add the --fast flag?

[ajm188]

config looks good to me. looks like vtctl.go isn't goimports'ed yet

[epk]

config looks good to me. looks like vtctl.go isn't goimports'ed yet

I don't know why it is failing in the CI, running the linter locally it passes.

It also passed on a previous commit with no changes to the file since :/

[shlomi-noach]

I don't know why it is failing in the CI, running the linter locally it passes.

Also passes for me, but I notice:

$ $(go env GOPATH)/bin/golangci-lint --version
golangci-lint has version 1.31.0 built from 3d6d0e7 on 2020-09-07T15:14:41Z

so I'm using 1.31.0, and CI presumably uses 1.34? @epk Can you verify what your own local golangci-lint version is?

[epk]

I don't know why it is failing in the CI, running the linter locally it passes.

Also passes for me, but I notice:

$ $(go env GOPATH)/bin/golangci-lint --version
golangci-lint has version 1.31.0 built from 3d6d0e7 on 2020-09-07T15:14:41Z

so I'm using 1.31.0, and CI presumably uses 1.34? @epk Can you verify what your own local golangci-lint version is?

I have the same version (1.34.1) locally:

~/src/vitess.io/vitess   epk/golangci-lint  $(go env GOPATH)/bin/golangci-lint --version
golangci-lint has version 1.34.1 built from d7dd233 on 2020-12-29T06:20:54Z
~/src/vitess.io/vitess   epk/golangci-lint  $(go env GOPATH)/bin/golangci-lint run go/...
~/src/vitess.io/vitess   epk/golangci-lint  echo $?
0

[shlomi-noach]

I have the same version (1.34.1) locally:

and, on the server side?

[shlomi-noach]

Have resolved merge conflicts and am further removing linter errors

[shlomi-noach]

Pinned golang-lint version to 1.31.0

[shlomi-noach]

Linter tests now passing