fix: lint #665
Open
fix: lint #665
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Re-enabled the dupl linter and refactored TestCheckResourceIntegration to eliminate code duplication by: - Creating helper functions for common nginx container spec and pod creation - Extracting shared expected object into a variable - Simplifying test cases to use these helpers 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com> Signed-off-by: Marcin Owsiany <porridge@redhat.com>
Re-enabled the gosmopolitan linter and fixed issues by adding nolint comments for legitimate Unicode test data in values_test.go. The Chinese characters (测试) are intentionally used to test Unicode support functionality. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com> Signed-off-by: Marcin Owsiany <porridge@redhat.com>
Re-enabled the nolintlint linter. No issues found - the previously reported issues appear to have been resolved by other changes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com> Signed-off-by: Marcin Owsiany <porridge@redhat.com>
Re-enabled the staticcheck linter and fixed 7 issues: - Simplified embedded field access by removing unnecessary ObjectMeta and TLSClientConfig selectors - Changed generic switch to tagged switch in serialization.go for better type safety - Updated event handling functions to directly access embedded fields 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com> Signed-off-by: Marcin Owsiany <porridge@redhat.com>
Signed-off-by: Marcin Owsiany <porridge@redhat.com>
The line length issues that were causing lll linter to be disabled have been resolved, allowing the linter to be re-enabled successfully. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com> Signed-off-by: Marcin Owsiany <porridge@redhat.com>
- Re-enabled errcheck linter in .golangci.yml - Fixed 10 errcheck violations across multiple files by properly handling unchecked error returns from Close() methods and os.RemoveAll() - Used //nolint:errcheck comments for defer cleanup operations where error handling would not be meaningful - Properly checked f.Close() error in tar.go where it matters for file integrity All errcheck violations are now resolved and linter passes successfully. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com> Signed-off-by: Marcin Owsiany <porridge@redhat.com>
Removed the temporary disable comment for revive linter. The linter was previously disabled due to 38 issues which will be addressed in subsequent commits. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com> Signed-off-by: Marcin Owsiany <porridge@redhat.com>
Added comprehensive package-level documentation following Go conventions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com> Signed-off-by: Marcin Owsiany <porridge@redhat.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com> f Signed-off-by: Marcin Owsiany <porridge@redhat.com>
Fixed revive unused-parameter warnings in dummy test functions by renaming unused 't *testing.T' parameters to '_ *testing.T'. These test functions exist only to satisfy coverage tools and don't use the testing parameter. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com> Signed-off-by: Marcin Owsiany <porridge@redhat.com>
Added targeted nolint:revive comments for package naming issues: - internal/http: conflicts with standard library 'http' package - internal/utils/docker.go: 'utils' package name considered meaningless Fixing these would require some more extensive changes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com> Signed-off-by: Marcin Owsiany <porridge@redhat.com>
Apparently which files get complained about is non-deterministic. Signed-off-by: Marcin Owsiany <porridge@redhat.com>
mclasmeier
reviewed
Jan 7, 2026
mclasmeier
reviewed
Jan 7, 2026
internal/harness/harness.go
Outdated
| } | ||
|
|
||
| defer f.Close() | ||
| defer f.Close() //nolint:errcheck |
Collaborator
There was a problem hiding this comment.
Wouldn't it be important to check for errors here? error on closing here could indicate that the config was not successfully written, no?
Member
Author
There was a problem hiding this comment.
Excellent point, I will change the code to propagate errors from Close on writable filehandles.
Member
Author
There was a problem hiding this comment.
I took a proper look at the Close() calls. It would be great if you could review the last commit @mclasmeier 🙏🏻
mclasmeier
reviewed
Jan 7, 2026
internal/http/client.go
Outdated
| return err | ||
| } | ||
| defer out.Close() | ||
| defer out.Close() //nolint:errcheck |
Collaborator
There was a problem hiding this comment.
Same question as before, is ignoring err here safe?
mclasmeier
reviewed
Jan 7, 2026
Collaborator
mclasmeier
left a comment
mclasmeier
left a comment
There was a problem hiding this comment.
I'm through. Just a few Close() questions, besides that lgtm.
Signed-off-by: Marcin Owsiany <porridge@redhat.com>
mclasmeier
approved these changes
Jan 8, 2026
| }, | ||
| }, w) | ||
| if err != nil { | ||
| w.Close() //nolint:errcheck // We do not care if closing failed in addition to writing, just preventing leaks. |
Collaborator
There was a problem hiding this comment.
nit, any particular reason why you are not using a deferred anonymous function, which does the Close() and assigns it to err in case err != nil? That way we wouldn't need to keep track of adding Close() calls to every early return.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
The recent bump of golangci-lint that I squeezed into #662 (which was mandatory due to the bumped go version) was somewhat rudimentary to keep the scope limited, and turned out to break the config a little. Me and Claude fixed it all in this PR. Reviewing by commit is highly recommended.