feat: automated go fix PR on Go version bump#4678
Conversation
|
Hi @copilot. Thanks for your PR. I'm waiting for a Azure member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
Co-authored-by: tuxerrante <8364469+tuxerrante@users.noreply.github.com>
go fix PR on Go version bump
Co-authored-by: tuxerrante <8364469+tuxerrante@users.noreply.github.com>
Code reviewFound 1 issue:
ARO-RP/.github/workflows/go-fix.yml Lines 5 to 12 in ebdf630 |
The project has two Go modules with independent go.mod files. The workflow now triggers on changes to both go.mod and pkg/api/go.mod, and checks version changes in both. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds automation to run go fix after Go version directive bumps and open a follow-up PR, while also applying the current go fix-driven build tag cleanup in existing e2e files.
Changes:
- Adds a
make go-fixtarget to rungo fix ./...in both the root module andpkg/api. - Introduces a GitHub Actions workflow to detect
godirective changes ingo.mod/pkg/api/go.mod, runmake go-fix, and open a PR with the results. - Applies
go fixoutput to e2e tests by removing legacy// +buildconstraints (keeping//go:build).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
Makefile |
Adds go-fix target to run go fix across both Go modules. |
.github/workflows/go-fix.yml |
New workflow to detect Go version bumps and automatically open a PR with go fix results. |
test/e2e/e2e_test.go |
Removes redundant legacy build tag line per go fix. |
test/e2e/aro.go |
Removes redundant legacy build tag line per go fix. |
test/e2e/aks.go |
Removes redundant legacy build tag line per go fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
incorporating both module versions in the branch name Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
- Add concurrency group to prevent parallel workflow conflicts - Use higher of the two module Go versions for setup-go - Include both module versions in branch name to avoid collisions - Replace --force-with-lease with --force (remote ref not fetched) - Make PR creation idempotent by checking for existing PR first - Output both root and api versions in PR description Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fixes appliedAddressed 5 of the 6 review comments from Copilot:
Not fixed (false positive): |
| group: go-fix | ||
| cancel-in-progress: false | ||
|
|
||
| permissions: |
There was a problem hiding this comment.
@mociarain do we have the rights for these permissions?
There was a problem hiding this comment.
They look correct to me but I'm sure what you are asking. Do you mean:
- Are these the correct permissions for the workflow?
- Do we have the rights to grant these permissions?
There was a problem hiding this comment.
The second one, I know these could be an issue:
contents: write
pull-requests: write
| exit 0 | ||
| fi | ||
| git commit -m "run go fix for Go ${{ steps.go-version.outputs.current }}" | ||
| git push --force origin "$BRANCH" |
There was a problem hiding this comment.
with the idempotent PR check we added, you could argue the workflow should just skip entirely if the branch already exists. We could replace the force push with a check-and-skip:
if git ls-remote --exit-code origin "refs/heads/$BRANCH" >/dev/null 2>&1; then
echo "Branch $BRANCH already exists, skipping."
exit 0
fi
git push origin "$BRANCH"
| @@ -0,0 +1,95 @@ | |||
| name: go-fix | |||
|
|
|||
| on: | |||
There was a problem hiding this comment.
This workflow can't be tested via normal CI because:
- It triggers on pushes to master that modify go.mod
- workflow_dispatch only works if the workflow file already exists on the default branch
To actually test this, you'd need to either:
- Fork the repo and merge the workflow into the fork's default branch, then trigger workflow_dispatch
- Create a test workflow on a branch that uses on: push to that branch instead of master (temporary, just for validation)
- Merge to master and test live (risky for a first run)
mociarain
left a comment
There was a problem hiding this comment.
Could we do a PR off this one that bumps something in each of the go.mods to prove it works? Or does it have to be merged first?
@mociarain I can try if it would start also when pushing onto another branch which is not master |
Made-with: Cursor
test: temporarily trigger go-fix on PR branch
test: trigger go-fix workflow via temporary pkg/api go bump
Made-with: Cursor
…revert test: revert temporary pkg/api go.mod bump
Made-with: Cursor
test: remove temporary pre-merge go-fix trigger
|
Pre-merge workflow validation update (second pass):
Conclusion: PR creation was NOT skipped for empty output in the Go 1.26 scenario; it failed due GitHub Actions permissions.
Current branch state (
Note: remote branch |
Made-with: Cursor
chore: bump harden-runner action to v2.16.1
…ation Made-with: Cursor
…enable test: temporarily enable PR-branch go-fix trigger for go1.26 validation
test: trigger go-fix non-empty path with temporary Go 1.26.1 bump
Made-with: Cursor
test: revert temporary Go 1.26.1 module bumps
Made-with: Cursor
…cleanup test: remove temporary PR-branch trigger after Go1.26 validation
Tracking Jira: ARO-25807
Adds a workflow to automatically open a PR running
go fixon both Go modules (github.com/Azure/ARO-RPandgithub.com/Azure/ARO-RP/pkg/api) whenever the Go version directive ingo.modis bumped onmaster.Changes
Makefile—go-fixtarget: Runsgo fix ./...on both modules, consistent with the existingfmtandlint-go-fixtargets:.github/workflows/go-fix.yml:pushtomasterwithpaths: ['go.mod'], plusworkflow_dispatchfor manual runsgodirective betweenHEADandHEAD~1to skip unrelatedgo.modchangesmake go-fix, then opens a PR using the GitHub CLI (gh pr create) on branchautomated/go-fix-<version>— no third-party actions required--force-with-leaseon push and skips PR creation gracefully ifgo fixproduces no changescontents: write+pull-requests: writepermissionstest/e2e/{aks,aro,e2e_test}.go: Appliedgo fixto the current codebase — removes the redundant pre-Go 1.17// +buildconstraint lines (superseded by//go:build).✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.