Skip to content

Commit

Permalink
Switch from golint to revive
Browse files Browse the repository at this point in the history
Golint is borderline abandonware (all that energy seems to be on `go vet` instead), and doesn't even ignore vendor folders from `./...` despite [having a github issue since 2017](golang/lint#320).
Plus it's non-configurable by design, which IMO encourages some bad habits.  See the next commit for details.

After a brief skim of the current state of things, `revive` popped out as a possibility, and the results are pretty good.
Runtimes on a fairly beefy machine:
```sh
# time make lint (before)
make lint  8.80s user 13.14s system 154% cpu 14.206 total
# time make lint (simply swapping bins)
make lint  10.85s user 16.89s system 149% cpu 18.545 total
# time make lint (new call)
make lint  21.72s user 4.22s system 2555% cpu 1.015 total
```
So that's a **14x reduction** in runtime, and now it's at the point where it's fine to run all the time (~1 second).
Locally, perhaps due to having fewer cores, this goes from 14s -> 2s.  Still substantially better, and still comfortably fast.

This default list also finds an additional 14 items, none major, and they basically match newer `golint` versions / what IDEs will often highlight:
```
# new lint, potentially worth following.
# there are many instances of this in the canary/ folder, but this was ignored earlier, so it is ignored now as well.
service/worker/failovermanager/starter.go:91:9: [context-keys-type] should not use basic type string as key in context.WithValue
# new lint, potentially worth following
service/worker/scanner/shardscanner/scanner_workflow.go:176:2: [if-return] redundant if ...; err != nil check, just return error instead.
# 3 new comment-formatting / exported-type-without-comment additions
```
Nothing significant, but (ignoring the new comment lints) possibly worth tackling, and not excessively noisy.

The following commits add some new lints + fixes, this one just aims for parity.
  • Loading branch information
Groxx authored Feb 1, 2021
1 parent b22be7c commit 2a0d88a
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 32 deletions.
13 changes: 4 additions & 9 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@ $(BIN)/enumer: go.mod | $(BIN)
$(BIN)/goimports: go.mod | $(BIN)
$(call get_tool,golang.org/x/tools/cmd/goimports)

$(BIN)/golint: go.mod | $(BIN)
$(call get_tool,golang.org/x/lint/golint)
$(BIN)/revive: go.mod | $(BIN)
$(call get_tool,github.com/mgechev/revive)

$(BIN)/protoc-gen-go: go.mod | $(BIN)
$(call get_tool,google.golang.org/protobuf/cmd/protoc-gen-go)
Expand Down Expand Up @@ -270,13 +270,8 @@ go-generate: $(BIN)/mockgen $(BIN)/enumer
@echo "updating copyright headers"
@$(MAKE) --no-print-directory copyright

lint: $(BIN)/golint fmt
@echo "running linter"
@lintFail=0; for file in $(sort $(LINT_SRC)); do \
$(BIN)/golint "$$file"; \
if [ $$? -eq 1 ]; then lintFail=1; fi; \
done; \
if [ $$lintFail -eq 1 ]; then exit 1; fi;
lint: $(BIN)/revive
$(BIN)/revive -config revive.toml -exclude './canary/...' -exclude './vendor/...' -formatter unix ./... | sort

fmt: $(BIN)/goimports $(ALL_SRC)
@echo "running goimports"
Expand Down
4 changes: 2 additions & 2 deletions cmd/server/tools.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ import (
_ "github.com/golang/mock/mockgen"
// enumer for generating utility methods for const enums
_ "github.com/dmarkham/enumer"
// golint - functional, but worth replacing with something less problematic and abandoned
_ "golang.org/x/lint/golint"
// replaces golint - configurable and much faster
_ "github.com/mgechev/revive"
// coverage reporting
_ "github.com/dmetzgar/goveralls"
)
10 changes: 3 additions & 7 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@ require (
github.com/dmetzgar/goveralls v0.0.3
github.com/eapache/go-resiliency v1.2.0 // indirect
github.com/emirpasic/gods v0.0.0-20190624094223-e689965507ab
github.com/fatih/color v0.0.0-20181010231311-3f9d52f7176a
github.com/fatih/structtag v1.1.0 // indirect
github.com/fatih/color v1.10.0
github.com/go-sql-driver/mysql v1.5.0
github.com/gocql/gocql v0.0.0-20191126110522-1982a06ad6b9
github.com/golang/mock v1.4.4
Expand All @@ -35,11 +34,9 @@ require (
github.com/m3db/prometheus_client_model v0.1.0 // indirect
github.com/m3db/prometheus_common v0.1.0 // indirect
github.com/m3db/prometheus_procfs v0.8.1 // indirect
github.com/mattn/go-colorable v0.0.9 // indirect
github.com/mattn/go-isatty v0.0.8 // indirect
github.com/mattn/go-runewidth v0.0.4 // indirect
github.com/mattn/go-sqlite3 v1.11.0 // indirect
github.com/olekukonko/tablewriter v0.0.1
github.com/mgechev/revive v1.0.3
github.com/olekukonko/tablewriter v0.0.4
github.com/olivere/elastic v6.2.21+incompatible
github.com/olivere/elastic/v7 v7.0.21
github.com/opentracing/opentracing-go v1.2.0
Expand All @@ -65,7 +62,6 @@ require (
go.uber.org/thriftrw v1.20.2
go.uber.org/yarpc v1.42.0
go.uber.org/zap v1.12.0
golang.org/x/lint v0.0.0-20200302205851-738671d3881b
golang.org/x/net v0.0.0-20201031054903-ff519b6c9102
golang.org/x/oauth2 v0.0.0-20201109201403-9fd604954f58
golang.org/x/time v0.0.0-20191024005414-555d28b269f0
Expand Down
32 changes: 19 additions & 13 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -113,11 +113,11 @@ github.com/envoyproxy/go-control-plane v0.9.9-0.20201210154907-fd9021fe5dad/go.m
github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c=
github.com/facebookgo/clock v0.0.0-20150410010913-600d898af40a h1:yDWHCSQ40h88yih2JAcL6Ls/kVkSE8GFACTGVnMPruw=
github.com/facebookgo/clock v0.0.0-20150410010913-600d898af40a/go.mod h1:7Ga40egUymuWXxAe151lTNnCv97MddSOVsjpPPkityA=
github.com/fatih/color v0.0.0-20181010231311-3f9d52f7176a h1:uGz8bS2tdMYpIjzS/ccMHV4H127Wz//pxlx7dN5qHB4=
github.com/fatih/color v0.0.0-20181010231311-3f9d52f7176a/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4=
github.com/fatih/color v1.10.0 h1:s36xzo75JdqLaaWoiEHk767eHiwo0598uUxyfiPkDsg=
github.com/fatih/color v1.10.0/go.mod h1:ELkj/draVOlAH/xkhN6mQ50Qd0MPOk5AAr3maGEBuJM=
github.com/fatih/structtag v1.0.0/go.mod h1:IKitwq45uXL/yqi5mYghiD3w9H6eTOvI9vnk8tXMphA=
github.com/fatih/structtag v1.1.0 h1:6j4mUV/ES2duvnAzKMFkN6/A5mCaNYPD3xfbAkLLOF8=
github.com/fatih/structtag v1.1.0/go.mod h1:mBJUNpUnHmRKrKlQQlmCrh5PuhftFbNv8Ys4/aAZl94=
github.com/fatih/structtag v1.2.0 h1:/OdNE99OxoI/PqaW/SuSK9uxxT3f/tcSZgon/ssNSx4=
github.com/fatih/structtag v1.2.0/go.mod h1:mBJUNpUnHmRKrKlQQlmCrh5PuhftFbNv8Ys4/aAZl94=
github.com/flynn/go-shlex v0.0.0-20150515145356-3f9db97f8568 h1:BHsljHzVlRcyQhjrss6TZTdY2VfCqZPbv5k3iBFa2ZQ=
github.com/flynn/go-shlex v0.0.0-20150515145356-3f9db97f8568/go.mod h1:xEzjJPgXI435gkrCt3MPfRiAkVrwSbHsst4LCFVfpJc=
github.com/fogleman/gg v1.2.1-0.20190220221249-0403632d5b90/go.mod h1:R/bRT+9gY/C5z7JzPU0zXsXHKM4/ayA+zqcVNZzPa1k=
Expand Down Expand Up @@ -276,25 +276,31 @@ github.com/m3db/prometheus_procfs v0.8.1 h1:LsxWzVELhDU9sLsZTaFLCeAwCn7bC7qecZcK
github.com/m3db/prometheus_procfs v0.8.1/go.mod h1:N8lv8fLh3U3koZx1Bnisj60GYUMDpWb09x1R+dmMOJo=
github.com/mailru/easyjson v0.7.6 h1:8yTIVnZgCoiM1TgqoeTl+LfU5Jg6/xL3QhGQnimLYnA=
github.com/mailru/easyjson v0.7.6/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJJLY9Nlc=
github.com/mattn/go-colorable v0.0.9 h1:UVL0vNpWh04HeJXV0KLcaT7r06gOH2l4OW6ddYRUIY4=
github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU=
github.com/mattn/go-isatty v0.0.8 h1:HLtExJ+uU2HOZ+wI0Tt5DtUDrx8yhUqDcp7fYERX4CE=
github.com/mattn/go-isatty v0.0.8/go.mod h1:Iq45c/XA43vh69/j3iqttzPXn0bhXyGjM0Hdxcsrc5s=
github.com/mattn/go-runewidth v0.0.4 h1:2BvfKmzob6Bmd4YsL0zygOqfdFnK7GR4QL06Do4/p7Y=
github.com/mattn/go-runewidth v0.0.4/go.mod h1:LwmH8dsx7+W8Uxz3IHJYH5QSwggIsqBzpuz5H//U1FU=
github.com/mattn/go-colorable v0.1.8 h1:c1ghPdyEDarC70ftn0y+A/Ee++9zz8ljHG1b13eJ0s8=
github.com/mattn/go-colorable v0.1.8/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc=
github.com/mattn/go-isatty v0.0.12 h1:wuysRhFDzyxgEmMf5xjvJ2M9dZoWAXNNr5LSBS7uHXY=
github.com/mattn/go-isatty v0.0.12/go.mod h1:cbi8OIDigv2wuxKPP5vlRcQ1OAZbq2CE4Kysco4FUpU=
github.com/mattn/go-runewidth v0.0.7 h1:Ei8KR0497xHyKJPAv59M1dkC+rOZCMBJ+t3fZ+twI54=
github.com/mattn/go-runewidth v0.0.7/go.mod h1:H031xJmbD/WCDINGzjvQ9THkh0rPKHF+m2gUSrubnMI=
github.com/mattn/go-sqlite3 v1.9.0/go.mod h1:FPy6KqzDD04eiIsT53CuJW3U88zkxoIYsOqkbpncsNc=
github.com/mattn/go-sqlite3 v1.11.0 h1:LDdKkqtYlom37fkvqs8rMPFKAMe8+SgjbwZ6ex1/A/Q=
github.com/mattn/go-sqlite3 v1.11.0/go.mod h1:FPy6KqzDD04eiIsT53CuJW3U88zkxoIYsOqkbpncsNc=
github.com/matttproud/golang_protobuf_extensions v1.0.1 h1:4hp9jkHxhMHkqkrB3Ix0jegS5sx/RkqARlsWZ6pIwiU=
github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0=
github.com/mgechev/dots v0.0.0-20190921121421-c36f7dcfbb81 h1:QASJXOGm2RZ5Ardbc86qNFvby9AqkLDibfChMtAg5QM=
github.com/mgechev/dots v0.0.0-20190921121421-c36f7dcfbb81/go.mod h1:KQ7+USdGKfpPjXk4Ga+5XxQM4Lm4e3gAogrreFAYpOg=
github.com/mgechev/revive v1.0.3 h1:z3FL6IFFN3JKzHYHD8O1ExH9g/4lAGJ5x1+9rPZgsFg=
github.com/mgechev/revive v1.0.3/go.mod h1:POGGZagSo/0frdr7VeAifzS5Uka0d0GPiM35MsTO8nE=
github.com/mitchellh/colorstring v0.0.0-20190213212951-d06e56a500db/go.mod h1:l0dey0ia/Uv7NcFFVbCLtqEBQbrT4OCwCSKTEv6enCw=
github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG+4E0Y=
github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0=
github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q=
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q=
github.com/modern-go/reflect2 v0.0.0-20180701023420-4b7aa43c6742/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3RllmbCylyMrvgv0=
github.com/modern-go/reflect2 v1.0.1/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3RllmbCylyMrvgv0=
github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U=
github.com/olekukonko/tablewriter v0.0.1 h1:b3iUnf1v+ppJiOfNX4yxxqfWKMQPZR5yoh8urCTFX88=
github.com/olekukonko/tablewriter v0.0.1/go.mod h1:vsDQFd/mU46D+Z4whnwzcISnGGzXWMclvtLoiIKAKIo=
github.com/olekukonko/tablewriter v0.0.4 h1:vHD/YYe1Wolo78koG299f7V/VAS08c6IpCLn+Ejf/w8=
github.com/olekukonko/tablewriter v0.0.4/go.mod h1:zq6QwlOf5SlnkVbMSr5EoBv3636FWnp+qbPhuoO21uA=
github.com/olivere/elastic v6.2.21+incompatible h1:QnTuofzxOCV5FrYLywjkMxOmOWhAeild1VXxKRksK9Y=
github.com/olivere/elastic v6.2.21+incompatible/go.mod h1:J+q1zQJTgAz9woqsbVRqGeB5G1iqDKVBWLNSYW8yfJ8=
github.com/olivere/elastic/v7 v7.0.21 h1:58a2pMlLketCsLyKg8kJNJG+OZIFKrSQXX6gJBpqqlg=
Expand Down Expand Up @@ -550,7 +556,6 @@ golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33/go.mod h1:STP8DvDyc/dI5b8T5h
golang.org/x/sys v0.0.0-20180909124046-d0be0721c37e/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20181116152217-5ac8a444bdc5/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20190222072716-a9d3bda3a223/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20190312061237-fead79001313/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20190403152447-81d4e9dc473e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
Expand All @@ -565,6 +570,7 @@ golang.org/x/sys v0.0.0-20191010194322-b09406accb47/go.mod h1:h1NjWce9XRLGQEsW7w
golang.org/x/sys v0.0.0-20191204072324-ce4227a45e2e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20191228213918-04cbcbbfeed8/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20200113162924-86b910548bc1/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20200116001909-b77594299b42/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20200122134326-e047566fdf82/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20200202164722-d101bd2416d5/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20200212091648-12a6c2dcc1e4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
Expand Down
2 changes: 1 addition & 1 deletion host/xdc/integration_failover_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1873,7 +1873,7 @@ func (s *integrationClustersTestSuite) TestCronWorkflowFailover() {
ExecutionStartToCloseTimeoutSeconds: common.Int32Ptr(100),
TaskStartToCloseTimeoutSeconds: common.Int32Ptr(1),
Identity: identity,
CronSchedule: common.StringPtr("@every 5s"),
CronSchedule: "@every 5s",
}
we, err := client1.StartWorkflowExecution(createContext(), startReq)
s.NoError(err)
Expand Down
29 changes: 29 additions & 0 deletions revive.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# config for https://github.com/mgechev/revive
ignoreGeneratedHeader = false
severity = "warning"
confidence = 0.8
errorCode = 0
warningCode = 0

#### roughly what golint does. probably only disable noisy ones.

[rule.blank-imports]
[rule.context-as-argument]
[rule.context-keys-type]
[rule.dot-imports]
[rule.error-naming]
[rule.error-return]
[rule.error-strings]
[rule.errorf]
[rule.exported]
[rule.if-return]
[rule.increment-decrement]
[rule.indent-error-flow]
[rule.package-comments]
[rule.range]
[rule.receiver-naming]
[rule.time-naming]
[rule.unexported-return]
[rule.var-declaration]
[rule.var-naming]

0 comments on commit 2a0d88a

Please sign in to comment.