-
Notifications
You must be signed in to change notification settings - Fork 1
Description
Here is something I wrote on a review
the settings you chose might seem pretty complete, while it isn't.
Many golangci-lint linters are using default rulesets that all pretty lax, and not strict.There are a lot of things to enable in govet, gocritict, staticcheck, revive to get interesting settings. Please note, not all of them are interesting. Some are in forcing rules barely no one want. Such as "never reuses a variable for error, define new one".
Discovering, enabling, and configuring linters is a journey, as interesting and perilous than going to the Mordor. You can lose yourself.
That's why I created my project of examples of golangci-lint configuration examples.
Originally posted by @ccoVeille in editorconfig-checker/editorconfig-checker#437 (comment)
Here is also a valuable comment about how to consider enabling the rules to a project:
I would say the strategy depends on the time you want to spend on fixing the issues golangci-lint may report.
I would say it's important to also do not block contributions to project.
There are many ways to achieve this.
Define a .golangci.yml pretty strict at the project root. This way IDE will report the issues. So core developers and advanced Go contributors who know golangci-lint will see what they need to fix.
Then you have multiple options:
- you define a relaxed golangci-lint configuration option for the CI, that can stay in .github folder.
So here, you have somehow to fix the issues reported by this configuration files before enforcing the CI to block them. Otherwise, you block contributions.
- second option is to play with
golangci-lint run -new
that can be used locally, but also configured on the golangci-lint-action via thewith
field.This feature is there to say "OK, old code sucks, we know that, but we want new code to be clean".
These options can be used at the sane time or separately
Everything is a matter of balance:
- the quality you want for your code.
- the amount of linters you want to use
- the amount of issues currently in the code base
- the amount of time you want to spend on fixing first batch of errors reported with a relaxed set of rules
- how much people will understand/agree the errors reported are qualitative and not noise, opinionated.
It seems to be complex, but the gain is real. Also, don't be afraid, it's a journey, time is needed, but there are multiple rewards in trying such a quest:
- working with golangci-lint rules was a huge opportunity for me. It learned a lot of valuable things in Go, but also as a developer/architect/mentor.
- I know a lot of these rules now. I write code that complies them by default now.
- the number of bugs I caught with the CI before they were merged is insane. People with less experience in Go, development skills, and unfortunately people who don't care about shipping "bad code that works", they will all face the same CI warning and blockers.
- the gain is important for code reviewers as the "obvious" errors are reported. So when you receive a PR, most little errors were caught and fixed before you review
- the gain is huge as a maintainer, it catches real bug that experienced reviewers can miss.
It's a very interesting and fascinating topic.
Originally posted by @ccoVeille in editorconfig-checker/editorconfig-checker#437 (comment)
Possible reviewers for the PR
- https://github.com/klaernie
- https://github.com/sepehrsoh
- https://github.com/mmorel-35
- https://github.com/Umang01-hash
- https://github.com/hippietrail
I don't use @ to avoid pinging them for now