chore: enable and fixes rules from gocritic - #246
Conversation
|
Overall LGTM, left some nits. |
28b23df to
1f2af12
Compare
| disabled-checks: | ||
| - appendAssign | ||
| - builtinShadow | ||
| - deferInLoop | ||
| - hugeParam | ||
| - importShadow | ||
| - paramTypeCombine | ||
| - rangeValCopy | ||
| - sloppyReassign | ||
| - unnamedResult | ||
| - whyNoLint |
There was a problem hiding this comment.
Is the intent to address all of these (in follow-ups), or are there that cannot (yet) be addressed (e.g. due to go versions).
It's good to leave a comment for each to outline if it's either a permanent (won't fix because) or temporary (TODO).
At least some of them stood out as "should likely be fixed" (whyNoLint, rangeValCopy, importShadow), and I know the builtinShadow is a bit too opinionated (it's "OK", but sometimes not worth the code-churn.
There was a problem hiding this comment.
Do you already know if you don't want some of them ? Otherwise the intention is indeed to try to comply with those that are the moby group and the sys project particularly usual practices.
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
kolyshkin
left a comment
kolyshkin
left a comment
There was a problem hiding this comment.
LGTM; @thaJeztah PTAL
4 participants
Enables all gocritic rules by default, then excludes the one non compliant then fix one rule per commit
Covers #245 gocritic part only