Create lint.yml

[willscott]

runs golangci-lint on commits.

Note: won't be enforced, but will provide warnings on commits to start us moving in the direction of maintaining a strictly linted codebase

ref: #161

[frrist]

Can you add a directive to the Makefile that runs linting as well? I'd like to be able to lint code before pushing it up.

[willscott]

Can you add a directive to the Makefile that runs linting as well? I'd like to be able to lint code before pushing it up.

golangci-lint generally wants to be installed globally rather than as something that can be run without side effects. Do you imagine make rules for the install, or just that assumes it already exists (make lint runs golangci-lint run ./...)?

[frrist]

Assuming it already exists for now should be fine, thanks.

[willscott]

the errors from the golangci-lint action are now the actual linting errors that currently exist in the code base

[frrist]

Way less than I was expecting:

  Error: Error return value of `release` is not checked (errcheck)
  Error: Error return value of `cleanup` is not checked (errcheck)
  Error: Error return value of `cleanup` is not checked (errcheck)
  Error: Error return value of `cleanup` is not checked (errcheck)
  Error: Error return value of `scheduler.Add` is not checked (errcheck)
  Error: Error return value of `scheduler.Add` is not checked (errcheck)
  Error: Error return value of `scheduler.Add` is not checked (errcheck)
  Error: Error return value of `w.Write` is not checked (errcheck)
  Error: Error return value of `proven.ForEach` is not checked (errcheck)
  Error: ineffectual assignment to `ctx` (ineffassign)
  Error: `tableName` is unused (structcheck)
  Error: `tableName` is unused (structcheck)
  Error: `tableName` is unused (structcheck)
  Error: S1011: should replace loop with `vmm = append(vmm, msgs.BlsMessages...)` (gosimple)
  Error: S1030: should use buf.String() instead of string(buf.Bytes()) (gosimple)
  Error: S1023: redundant `return` statement (gosimple)
  Error: lostcancel: the cncl function is not used on all paths (possible context leak) (govet)
  Error: lostcancel: this return statement may be reached without using the cncl var defined on line 70 (govet)
  Error: SA4021: x = append(y) is equivalent to x = y (staticcheck)
  Error: SA5008: duplicate struct tag "pg" (staticcheck)
  Error: SA1019: peer.ID is deprecated: use github.com/libp2p/go-libp2p-core/peer.ID instead.  (staticcheck)
  Error: SA1019: package github.com/libp2p/go-libp2p-peer is deprecated: use github.com/libp2p/go-libp2p-core/peer instead.  (staticcheck)
  Error: SA1019: peer.ID is deprecated: use github.com/libp2p/go-libp2p-core/peer.ID instead.  (staticcheck)
  Error: SA1019: package github.com/libp2p/go-libp2p-peer is deprecated: use github.com/libp2p/go-libp2p-core/peer instead.  (staticcheck)
  Error: SA1019: peer.ID is deprecated: use github.com/libp2p/go-libp2p-core/peer.ID instead.  (staticcheck)
  Error: SA1019: package github.com/libp2p/go-libp2p-peer is deprecated: use github.com/libp2p/go-libp2p-core/peer instead.  (staticcheck)

[willscott]

it looks like there are actually more lint issues than this, the tool just stop after some point.

We have a couple options here:

• we can merge this and have the ci check fail on PRs until linting is clean up throughout the code base
• we can limit which folders we lint, and do a series of PRs linting different components and adding them to lint coverage
• we can work through all the linting in this PR

preferences?

[placer14]

For CI at least, there's an option which allows only new lint errors after a certain commit to be triggered. This lets you enable the linter for new changes and gradually fix the old ones over time. --new-from-rev REV

[willscott]

Rebased to master

[frrist]

My approval is sticky assuming lint failures do not block merges.