-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
chore(linters): Fix remaining errcheck warnings #15518
Conversation
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.
Thank you for doing this!! very excited about getting this turned on. I have a few spots with small changes and two questions about error messages.
plugins/inputs/webhooks/artifactory/artifactory_webhook_test.go
Outdated
Show resolved
Hide resolved
Co-authored-by: Joshua Powers <powersj@fastmail.com>
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.
Thanks @DStrand1 for the epic effort! A few comments/suggestions from my side...
plugins/inputs/burrow/burrow_test.go
Outdated
//nolint:errcheck // should fail tests later on if this read fails | ||
b, _ := os.ReadFile(jsonFile) | ||
return b, code |
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.
I think you should return the error here. The issue is that if read fails (e.g. we specified a wrongly named file) the tests can pass when returning an empty response...
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.
Since this function was called nested a few functions deep in test cases with no access to testing.T
, I decided to panic here if there is an error instead
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.
Thanks @DStrand1! Just one (actually two) suggestions left...
Download PR build artifacts for linux_amd64.tar.gz, darwin_arm64.tar.gz, and windows_amd64.zip. 📦 Click here to get additional PR build artifactsArtifact URLs |
Co-authored-by: Joshua Powers <powersj@fastmail.com> (cherry picked from commit 19737fc)
Summary
Fixes remaining errcheck warnings in input/output plugin directories and removes remaining temporary .golangci.yml exclusions
Checklist
Related issues
resolves #13013