NO-JIRA: chore: bump golangci-lint to v2.13.1 - #393
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@jmelis: This pull request explicitly references no jira issue. DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci
Bot
added
the
approved
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift-online/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
Included review availability: Your plan provides up to 12 included reviews per hour; 10 remain after this review. WalkthroughThe controllers now rely on finalizer update watch events instead of explicit requeues. Placement retries ChangesController reconciliation behavior
Go linting toolchain updates
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The dependency and lint configuration updates are localized, with no actionable merge-blocking risk remaining beyond normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 10 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (10 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 7 files. (1 skipped: 1 unsupported.) Full details: No-Weak-CryptoExplanation PASS: The pull-request diff adds no MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB, custom crypto, or secret-comparison code. The changed Go files only modify reconciliation results and add a time import. Existing SHA-1 code in Full details: Container-PrivilegesExplanation PASS. The pull request changes only Go module files and Go controller/test files. It adds no Kubernetes or container manifest files, and no added line contains privileged, hostPID, hostNetwork, hostIPC, SYS_ADMIN, or allowPrivilegeEscalation settings. Existing manifests show restrictive settings such as runAsNonRoot: true and allowPrivilegeEscalation: false; they are unchanged. Full details: No-Sensitive-Data-In-LogsExplanation No logging changes were introduced. The diff changes requeue behavior, tests, imports, and tooling dependencies only. Existing controller log calls and fields remain unchanged, and no added line logs passwords, tokens, API keys, PII, session IDs, hostnames, or customer data. Full details: No-Hardcoded-SecretsExplanation No hardcoded secret was introduced. The application changes add only controller comments, Full details: No-Injection-VectorsExplanation PASS. The pull-request diff only updates Go lint-tool dependencies and controller requeue behavior/tests. Focused searches of all changed source files found no SQL concatenation, shell=True, eval/exec on untrusted data, pickle.loads, yaml.load without SafeLoader, os.system with variables, or dangerouslySetInnerHTML with user data. The repository-wide exec.Command matches are pre-existing Go test/tooling code and are not introduced by this pull request. Full details: Ai-AttributionExplanation AI use is explicit: the PR description says it was generated with Claude Code, and the sole PR commit contains
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Some tools did not complete. Review the errors below. 🔧 golangci-lint (2.12.2)level=error msg="Running error: context loading failed: no go files to analyze: running Comment |
Bumps golangci-lint to v2.13.1 (with the gofmt digest it requires),
which ships a stricter staticcheck that flags SA1019 for the deprecated
ctrl.Result{Requeue: true}.
Replaces the four usages:
- Finalizer-add sites (cluster, manifest, nodepool): return an empty
ctrl.Result{}. The finalizer Update emits a watch event that
re-enqueues the object, so no explicit requeue is needed.
- Placement AlreadyExists site: the Create failed so no watch event is
emitted; requeue explicitly with RequeueAfter: 5s.
Updates the three finalizer unit tests to assert no explicit requeue
instead of the deprecated Requeue field.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2aac06e to
22b2216
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jmelis, psav The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
3 participants
Summary
Bumps golangci-lint to v2.13.1 and the gofmt digest it requires, folding in the linter-ecosystem Renovate PRs (#375, #379, #363, #367, #373).
The new golangci-lint ships a stricter staticcheck that flags SA1019 for the deprecated
ctrl.Result{Requeue: true}(controller-runtime). This PR fixes the four usages:ctrl.Result{}. The finalizerUpdateemits a watch event that re-enqueues the object (the controllers useFor(&T{})with no predicates), so no explicit requeue is needed — this is the idiomatic controller-runtime pattern.AlreadyExistssite: theCreatefailed so no watch event is emitted; requeue explicitly withRequeueAfter: 5 * time.Second(matching the existing convention in these controllers).The three finalizer unit tests are updated to assert no explicit requeue instead of the deprecated
Requeuefield.Test plan
make build✅make lint— 0 issues across all modules (previously 4 SA1019 findings, 3 hidden bymax-same-issues) ✅make test— full suite green ✅Closes / supersedes
Covers the golangci-lint-ecosystem Renovate PRs: #375, #379, #363, #367, #373.
🤖 Generated with Claude Code
Summary by CodeRabbit
Improvements
Maintenance
Tests