Golangci-linters - what we should have enabled?

[tuminoid]

In Metal3, golangci-linter works by disabling all linters, and then white-listing the ones that pass.

We have couple issues about enabling more golangci-linters. We have 40+ in CAPM3 and BMO, 30+ in IPAM and 30+ now in IRSO.

• Increase number of enabled golangci-lint linters ip-address-manager#752
• Increase number of enabled golangci-lint linters  cluster-api-provider-metal3#2106
• Increase number of enabled golangci-lint linters  #2072
• IRSO TBD

There are 50 golangci-linters available white enable-all: true, with 100+ in total. We should have a discussion which ones we aim to enable, and then achieve that in all repositories under Metal3. We should also look at the exceptions we've accumulated over the years, and decide if they should be dropped, and if not, document them better.

Based on the results of this discussion, I will adjust the linked issues and create new ones as needed.

[lentzi90]

If we are anyway at ~40 % enabled, how about we switch to enable all and then disable those that fail? Then it will be more obvious what we have disabled and we can create specific issues for fixing and enabling those that we care about.

[tuminoid]

I tried this, and actually it says there are 50 enabled when using enable-all: true in the config. 100+ is mentioned in golangci-lint, so the rest must be manually white-listed to enable. I revised the expectation to the 50 linters, not 100+.

Gist with output from enable-all: true: https://gist.github.com/tuminoid/734f11b6ffc7912e729458e2e6acbf86

In CAPM3, we fail:

• dupl
• funlen
• gochecknoinits
• godox
• gofumpt
• gomoddirectives
• lll
• nestif
• depguard
• forcetypeassert
• gomnd
• inamedparam
• interfacebloat
• intrange
• nlreturn
• testpackage
• wsl

I think we fail all that we have not enabled. I agree we should make it explicit in the config. Some of the errors seem fixable, some are quite intrusive and not productive.

[elfosardo]

@lentzi90 I actually went for the opposite in IrSO in metal3-io/ironic-standalone-operator#94 but I agree with that, I'm still in time to change it in case we want to experiment there as the lint test is not active yet

[tuminoid]

IRSO implementation was OK as is. We just need to enable the rest of the linters, and then add some more based on what we agree here. We did take a look at the failure list, and many of them are fixable, but some of them like wsl go way overboard.

We're not in hurry here, so I'll take time at some point to dig into each failing one, and suggest enabling/keeping disabled for each of them, with preferred fix suggestions, to decide.