HYPERFLEET-1334 - chore: replace bingo with Go 1.24 tool directives#143
HYPERFLEET-1334 - chore: replace bingo with Go 1.24 tool directives#143kuudori wants to merge 5 commits into
Conversation
|
Skipping CI for Draft Pull Request. |
|
[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 |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change replaces Bingo-generated tool configuration with a Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Makefile
participant ToolsModule
participant GolangCILint
Makefile->>ToolsModule: Run golangci-lint with tools/go.mod
ToolsModule->>GolangCILint: Resolve pinned tool
GolangCILint->>Makefile: Lint ./...
🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
eca3492 to
62a1077
Compare
33ee306 to
b036021
Compare
b036021 to
68a4ea2
Compare
68a4ea2 to
834776f
Compare
d279f54 to
db9475a
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Makefile`:
- Around line 130-132: Update the Makefile targets verify and/or check so the
default verification flow invokes verify-tools, ensuring stale tools/go.mod or
tools/go.sum files fail normal make check execution. Preserve the existing
verification steps and dependency structure while wiring verify-tools into the
gate.
- Around line 5-6: Update the Makefile’s GO usage in the gotool helper and
tidy-tools recipe so GO is treated as one executable path rather than raw shell
syntax. Quote or otherwise validate the GO value before expansion, while
preserving the existing tool and tidy behavior.
- Around line 5-6: Update the gotool Makefile definition to export GOWORK=off
when invoking go tool, matching the environment handling used by tidy-tools.
Preserve the existing TOOL_MOD and argument forwarding so commands such as lint
can run successfully outside workspace mode.
In `@tools/go.mod`:
- Line 7: Update the github.com/oapi-codegen/oapi-codegen/v2 dependency in
tools/go.mod from v2.5.1 to v2.7.1, then regenerate tools/go.sum so it reflects
the upgraded module and checksums.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 73cc2ea7-5ddb-4251-8180-f05718a8419e
⛔ Files ignored due to path filters (1)
tools/go.sumis excluded by!**/*.sum,!**/go.sum
📒 Files selected for processing (8)
.bingo/.gitignore.bingo/README.md.bingo/Variables.mk.bingo/go.mod.bingo/golangci-lint.mod.bingo/variables.envMakefiletools/go.mod
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift-hyperfleet/architecture(manual)openshift-hyperfleet/hyperfleet-api(manual)openshift-hyperfleet/hyperfleet-sentinel(manual)openshift-hyperfleet/hyperfleet-adapter(manual) → reviewed against branchchore/replace-bingo-with-go-tool-directivesinstead of the default branchopenshift-hyperfleet/hyperfleet-broker(manual) → reviewed against open PR#26chore/replace-bingo-with-go-tool-directivesinstead of the default branch
💤 Files with no reviewable changes (6)
- .bingo/README.md
- .bingo/.gitignore
- .bingo/golangci-lint.mod
- .bingo/go.mod
- .bingo/Variables.mk
- .bingo/variables.env
f5f69b1 to
88ac768
Compare
88ac768 to
1e07afd
Compare
- Quote Make variables in gotool and tool recipes (CWE-78) - Compare tool files against HEAD in verify-tools
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@AGENTS.md`:
- Line 17: Update the make check entry in AGENTS.md to include verify-tools in
the documented sequence, matching the Makefile implementation while preserving
the existing generate, fmt-check, vet, lint, and test steps.
In `@Makefile`:
- Around line 130-132: Update the verify-tools target to detect untracked
tools/go.mod and tools/go.sum files using git ls-files --others
--exclude-standard, and fail with the existing out-of-date guidance when either
manifest is untracked. Preserve the current git diff validation for tracked
manifest changes.
- Around line 126-132: Expose a `tidy-tools` Make target for the tool-module
tidy operation currently defined by `tools`, either by renaming it or adding an
alias. Update `verify-tools` to depend on `tidy-tools` while preserving its
existing drift check and failure message.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 887a0ded-59af-48b4-9ba7-8266b071cf44
⛔ Files ignored due to path filters (1)
tools/go.sumis excluded by!**/*.sum,!**/go.sum
📒 Files selected for processing (9)
.bingo/.gitignore.bingo/README.md.bingo/Variables.mk.bingo/go.mod.bingo/golangci-lint.mod.bingo/variables.envAGENTS.mdMakefiletools/go.mod
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift-hyperfleet/architecture(manual)openshift-hyperfleet/hyperfleet-api(manual)openshift-hyperfleet/hyperfleet-sentinel(manual)openshift-hyperfleet/hyperfleet-adapter(manual) → reviewed against branchchore/replace-bingo-with-go-tool-directivesinstead of the default branchopenshift-hyperfleet/hyperfleet-broker(manual) → reviewed against open PR#26chore/replace-bingo-with-go-tool-directivesinstead of the default branch
💤 Files with no reviewable changes (6)
- .bingo/.gitignore
- .bingo/README.md
- .bingo/variables.env
- .bingo/golangci-lint.mod
- .bingo/go.mod
- .bingo/Variables.mk
🚧 Files skipped from review as they are similar to previous changes (1)
- tools/go.mod
a617dce to
375b4fd
Compare
|
@kuudori: The following test failed, say
Full PR test history. Your PR dashboard. 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. |
HYPERFLEET-1334
Replace Bingo / standalone
go installwith a singletools/go.modusing Go 1.24+ tool directives.Tools are pinned in
tools/go.modand invoked via$(call gotool,<name>)in the Makefile.Tools:
Also adds
tidy-toolsandverify-toolsMake targets.Follow-up changes (review comment fixes)
$(GO)and$(TOOL_MOD)in Makefilegotooland tool recipes to prevent shell injection (CWE-78)verify-toolsnow compares againstHEAD(git diff HEAD --) instead of the working tree