Onboard CodeRabbit and enable golangci-lint standard linters#5132
Onboard CodeRabbit and enable golangci-lint standard linters#5132dhiller wants to merge 8 commits into
Conversation
As a precondition for enabling golangci-lint goimports formatter we reformat source files with import ordering issues. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Daniel Hiller <dhiller@redhat.com>
Re-enable errcheck linter and fix all warnings: use deferred closures for file/response body Close() calls, suppress fmt.Fprint* to stdout/stderr with nolint:errcheck, check fs.Parse/tmpl.Execute/ os.WriteFile return values, and use blank identifier for cleanup operations like os.RemoveAll. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Daniel Hiller <dhiller@redhat.com>
Switch from default: none with explicit linter list to default: standard which includes errcheck, govet, ineffassign, staticcheck, and unused. Re-add gocyclo via enable: since it is not part of the standard set. Add goimports formatter for automatic import ordering. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Daniel Hiller <dhiller@redhat.com>
Add CodeRabbit configuration with "chill" profile and non-blocking reviews. Enable golangci-lint integration and add path-specific review instructions for Go code, tests, Prow job configs, and shell scripts. This is a prerequisite of replacing Gemini Code Assist (which will be sunset) with CodeRabbit. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Daniel Hiller <dhiller@redhat.com>
The original code intentionally ignored the generateReleaseNotes error since release notes generation failing should not block tag creation. Change from hard error to warning log to preserve this behavior. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Daniel Hiller <dhiller@redhat.com>
|
Skipping CI for Draft Pull Request. |
kubevirt-bot
added
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request systematically addresses unhandled errors across the codebase by explicitly ignoring them, adding inline linter bypasses, or properly handling deferred cleanups. The review feedback highlights several critical areas for improvement: first, ignoring errors on deferred Close() calls for GCS writers can lead to silent upload failures, so these should be explicitly checked on success paths. Second, rather than scattering //nolint:errcheck comments across dozens of fmt.Fprint* calls, the linter should be configured globally in .golangci.yaml. Finally, the feedback addresses inconsistent error handling in release-tool.go and corrects an anti-pattern in org-access-check/main.go where os.WriteFile is used on an already open temporary file.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
- Add errcheck exclude-functions for fmt.Fprint/Fprintf/Fprintln in .golangci.yaml and remove all inline //nolint:errcheck comments - Check GCS writer Close() errors on success path in pkg/mirror/bazel.go since Close() performs the actual upload - Check GCS writer Close() errors via named return in robots/flakefinder/report.go for all three report writers - Use writeString helper in releng/release-tool for consistent error handling across all WriteString calls - Fix os.WriteFile anti-pattern in robots/org-access-check: use temp.Write() on the already-open file handle instead Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Daniel Hiller <dhiller@redhat.com>
|
/gemini please review again |
There was a problem hiding this comment.
Code Review
This pull request introduces widespread improvements to error handling, resource cleanup, and linter compliance across the codebase, including wrapping deferred close operations, explicitly ignoring unhandled errors, and updating the static analysis configurations. The review feedback correctly highlights two critical issues where defer is used inside for loops in pkg/mirror/bazel.go and robots/per-test-execution/main.go, which could lead to resource leaks and file descriptor exhaustion.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
/gemini review |
Replace deferred Close() calls inside for loops with explicit close on each exit path, preventing accumulation of unclosed resources across loop iterations. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Daniel Hiller <dhiller@redhat.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces widespread updates across the codebase to improve error handling, resource cleanup, and linter compliance. Key changes include updating the .golangci.yaml configuration, explicitly ignoring unused return values (such as _ = fs.Parse(...) and _ = writer.Write(...)), wrapping deferred close operations in anonymous functions to ignore their errors, and adding proper error assertions in tests. Feedback on these changes highlights two issues: a potential connection leak in pkg/flake-stats/report.go if the HTTP status check fails before the body is deferred closed, and a potential nil pointer dereference panic in external-plugins/referee/metrics/metrics_test.go if the response is nil when the test times out.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Code Review
This pull request introduces widespread improvements across the codebase to address unhandled errors, explicitly ignore discarded errors, and ensure proper resource cleanup, alongside updates to the linter configurations. The review feedback highlights critical issues where calling Close() on GCS writers during error paths or checksum mismatches commits corrupted or partial data to Google Cloud Storage; it is recommended to use CloseWithError(err) to safely abort these writes. Additionally, the feedback suggests wrapping a loop body in an immediately invoked function expression (IIFE) to manage file resources safely with defer rather than relying on fragile manual close calls before multiple return statements.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
- Use context cancellation to abort GCS writer uploads on error paths instead of Close() which commits partial/corrupt data (bazel.go, report.go, stats.go) - Check GCS writer Close() error on success path (stats.go) - Move resp.Body.Close() defer before status check to prevent connection leak on non-200 responses (flake-stats/report.go) - Add nil check on resp before closing body in test (metrics_test.go) - Extract readTopXFromFile helper so defer scopes correctly per iteration instead of accumulating in loop (per-test-execution) Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Daniel Hiller <dhiller@redhat.com>
kubevirt-bot
removed
the
do-not-merge/work-in-progress
|
@dhiller: The following test failed, say
DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
/uncc @rthallisey |
|
/test pull-project-infra-ci-search-deploy-test 🤖 Assisted by Claude |
|
@dhiller: The following test failed, say
DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
2 participants
What this PR does / why we need it:
Port of kubevirt/ci-health#150 to project-infra:
.coderabbit.yamlwith "chill" profile, non-blocking reviews, and path-specific instructions for Go, tests, Prow job YAML, and shell scripts.golangci.yamlfromdefault: nonetodefault: standard(addserrcheck; keepsgocyclovia explicit enable)goimportsformatter to golangci-lint configThis is a prerequisite of replacing Gemini Code Assist (which will be sunset) with CodeRabbit.
Special notes for your reviewer:
golangci-lint run ./...passes with 0 issuesgo test ./...passes for all packages🤖 Assisted by Claude