-
Notifications
You must be signed in to change notification settings - Fork 2.3k
CI: bump github actions, simplify things #1476
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
Changes from all commits
ddfc115
86bd356
edc8467
f9a9289
9d0e395
a0220c3
2a50399
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,7 +6,6 @@ linters: | |
| - asasalint | ||
| - asciicheck | ||
| - bidichk | ||
| - bodyclose | ||
| - contextcheck | ||
| - durationcheck | ||
| - errchkjson | ||
|
|
@@ -22,46 +21,10 @@ linters: | |
| - nilerr | ||
| - nilnesserr | ||
| - noctx | ||
| - protogetter | ||
| - reassign | ||
| - recvcheck | ||
| - rowserrcheck | ||
| - spancheck | ||
| - sqlclosecheck | ||
| - testifylint | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps we should enable this one for tests though; I recall there was a PR that fixed issues reported by it, so would be good to have it run in CI to make sure we don't regress;
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's address this as a followup -- the focus of this one is to simplify things / remove useless configuration. If I add some linters for test files, this PR will probably grow much bigger (and harder to review). I just checked, if we lint test files (and leave testifylint), we get 19 issues, 7 of those for testifylint. I can fix all those but not in this PR.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK I have three more commits that enable some linters for test files (including testifylint) and fix some of their warnings. Let me know if you want those commits here in this PR @thaJeztah
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's bring this PR in; ok or a follow-up 👍 |
||
| - unparam | ||
| - zerologlint | ||
| disable: | ||
| - prealloc | ||
kolyshkin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| settings: | ||
| errcheck: | ||
| check-type-assertions: false | ||
| check-blank: false | ||
| lll: | ||
| line-length: 100 | ||
| tab-width: 4 | ||
| prealloc: | ||
| simple: false | ||
| range-loops: false | ||
| for-loops: false | ||
| whitespace: | ||
| multi-if: false | ||
| multi-func: false | ||
| exclusions: | ||
| generated: lax | ||
| presets: | ||
| - comments | ||
| - common-false-positives | ||
| - legacy | ||
| - std-error-handling | ||
| paths: | ||
| - third_party$ | ||
| - builtin$ | ||
| - examples$ | ||
| formatters: | ||
| exclusions: | ||
| generated: lax | ||
| paths: | ||
| - third_party$ | ||
| - builtin$ | ||
| - examples$ | ||
This file was deleted.
This file was deleted.
This file was deleted.
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.
Should we enable linting on tests? I agree that in this codebase,
bodycloseis less useful, then again, I've seen it catch some things that were overlooked, and the codebase is relatively small so not a huge concern to run more linters?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.
see #1476 (comment)