-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Lint] Change to linter deny list and fix formerly excluded linters #586
Conversation
From my point of view, this PR is currently on stale, besides the good intent (and our general desire to proceed with this topic). I added it to the project and would suggest to continue it as part of our current maintenance / bug fixing efforts. |
67c9999
to
84dfd50
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #586 +/- ##
==========================================
- Coverage 76.92% 76.88% -0.05%
==========================================
Files 41 41
Lines 3498 3491 -7
==========================================
- Hits 2691 2684 -7
- Misses 592 594 +2
+ Partials 215 213 -2 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many thanks for this tedious work -- I think it is a really huge improvement! 👏
It's also great to see that you were creating so many commits, which made it rather easy for me to review the changes on a commit-by-commit bases. Also, the comments (especially when disabling a linter or on other decisions) were helpful. Overall, I reviewed all commits individually, and learned more about Golang linters during some web searchers 🙂
At some point, the Linter threw errors that were incomprehensible.
because we are currently using dependabot and manual checking of the go.mod file to keep our supply chain secure. A separate handling via a debguard configuration would at the current point not introduce more security compared to the added complexity.
because it feels wrong that we would have to exhaustively define all switch cases even if a default definition is present.
because it feels wrong to exhaustively initialize structs fields with nil.
because we have too many variables that are almost constants and, thus, this linter is too imprecise.
because we currently have to use the `replace` directive.
because due to lacking understanding of the advantages. Especially for our storage object, this is exactly required.
as we are used to do differently.
as we use the named returns to further indicate the semantics.
until we address this enhancement with #631.
because for our unit tests we do not follow the black box approach. The black box approach is covered by our e2e tests.
even though it is useful for new projects, we already have established many abbreviations that would cause more work to change that benefits.
because we use newlines for semantic purposes and not only rule-based like this linter.
84dfd50
to
9b6855d
Compare
No description provided.