status-im / status-go

The Status module that consumes go-ethereum
https://status.im
Mozilla Public License 2.0
728 stars 245 forks source link

Replace gometalinter with golangci-lint #1017

Closed pedropombeiro closed 6 years ago

pedropombeiro commented 6 years ago

Problem

gometalinter is very slow compared to golangci-lint. In order to speed up CI builds, we should migrate to the faster tool.

Context: https://status-im.slack.com/archives/C8QP8S5UH/p1527617461000018

time make lint
lint

real    1m4.286s
user    3m21.009s
sys     0m20.387s

Implementation

Acceptance Criteria

The new tool runs in a functionally equivalent way to gometalinter. The new tool is faster than the existing solution.

pedropombeiro commented 6 years ago

Took a preliminary look at golangci-lint and found the following differences compared to gometalinter:

real    0m14.062s
user    0m43.052s
sys     0m4.261s

On the other hand, it seems to produce more useful results than gometalinter. An example: image

@divan @adambabik do you have any suggestions regarding the above limitations? E.g. should we run the misspell linter separately in the make lint target? gasv2 linter is just gas -fmt=csv, is this required?

adambabik commented 6 years ago

In my opinion, gas is still too experimental and I would get rid of it. We can run misspell individually, not a problem. As long as it will be encapsulated in make lint target.