Fix bump-go.sh to tolerate missing toolchain directive#12581
Merged
Conversation
Copilot
AI
changed the title
[WIP] Update bump-go.sh to handle missing toolchain gracefully
Fix bump-go.sh to tolerate missing toolchain directive
Jan 29, 2026
Member
|
@copilot can you rewrite this using |
Contributor
Author
Done! Rewrote the script to use Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
babakks
reviewed
Jan 29, 2026
babakks
reviewed
Jan 29, 2026
- Add early trap setup to avoid exit code issues - Handle missing toolchain directive gracefully with `|| true` - Add logic to detect when toolchain is expected to be missing - Add informative messages about missing toolchain - Implement smart toolchain handling: - Skip toolchain when go version matches latest (redundant) - Add toolchain when go version is older than latest - Update toolchain when it exists but is outdated Co-authored-by: williammartin <1611510+williammartin@users.noreply.github.com>
- Compare major.minor versions instead of exact string match - Preserve go directive if already at latest major.minor - Handle cases where go.mod has full patch version (e.g., 1.25.6) - Update PR body to use actual go directive from modified go.mod - Add detailed comments explaining version handling logic Co-authored-by: williammartin <1611510+williammartin@users.noreply.github.com>
- Clarify comment to say "matches" instead of "matches or exceeds" - Update PR body to accurately reflect final go.mod state - Show actual toolchain if present, or note it's not specified - Provide clear explanation when toolchain is omitted Co-authored-by: williammartin <1611510+williammartin@users.noreply.github.com>
Replace manual parsing and editing with go mod edit: - Use 'go mod edit -json' to read current go and toolchain directives - Use 'go mod edit -go' to update go directive - Use 'go mod edit -toolchain' to update toolchain directive - Remove manual sed/grep parsing and .bak file handling - More reliable and maintainable than custom text manipulation Co-authored-by: williammartin <1611510+williammartin@users.noreply.github.com>
The jq -r flag with // "" operator already returns empty string for null/missing values, never the literal string "null". The checks for == "null" are unnecessary. Co-authored-by: babakks <36728931+babakks@users.noreply.github.com>
b7fd470 to
d32b18e
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Improves the Go bump automation script to better handle go.mod files that may not contain a toolchain directive, moving parsing/editing to go mod edit -json + jq and adjusting branch cleanup/PR body generation.
Changes:
- Replace
grep/sedparsing withgo mod edit -json+jq, and updatego.modviago mod edit -go/-toolchain. - Add early “up-to-date” detection that tolerates missing
toolchain, and guard branch cleanup with aBRANCH_CREATEDflag. - Build PR body from the post-edit
go.modstate (including a “toolchain not specified” case).
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/scripts/bump-go.sh | Switches to go mod edit-based parsing/editing and adjusts toolchain/up-to-date logic, cleanup handling, and PR body generation. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (3)
.github/workflows/scripts/bump-go.sh:99
NEW_GO_DIRECTIVEis forced toX.Y.0, but this repository’sgo.modcurrently uses a patch-levelgodirective (e.g.go 1.25.6). Forcing.0will change both style and semantics (minimum required patch), and it also makes the “toolchain becomes redundant” path less likely to occur. Consider preserving the existinggodirective form (patch vs.0) or setting it to the target toolchain patch version.
git add "$GO_MOD"
[[ -f "$GO_SUM" ]] && git add "$GO_SUM"
# ---- Commit -----------------------------------------------------------------
COMMIT_MSG="Bump Go to $TOOLCHAIN_VERSION"
git commit -m "$COMMIT_MSG" >/dev/null
.github/workflows/scripts/bump-go.sh:153
- PR description mentions switching the PR search to
--state openand using$GITHUB_REPOSITORYinstead of a hardcoded repo, but the script still runsgh search prs --repo cli/cliwithout--state open(later in the file). Either update the implementation to match the description or adjust the PR description to avoid documenting changes that aren’t present.
.github/workflows/scripts/bump-go.sh:169 - In the no-toolchain PR body path, the text says “go version matches latest toolchain
X.Y.Z”, but the script’s logic treats a major.minor match as “latest” and may set thegodirective toX.Y.0. That message can be misleading (especially for patch releases). Consider either bumping thegodirective to the full patch version or rewording this line to explicitly state it’s matching onlymajor.minor.
- Files reviewed: 1/1 changed files
- Comments generated: 2
babakks
approved these changes
May 13, 2026
Address review feedback: always set both go and toolchain directives via go mod edit, then let go mod tidy normalize. This eliminates complex conditional toolchain handling. Additional fixes: - Add go mod tidy after edits to reconcile dependencies - Commit go.sum alongside go.mod - Filter PR search to open PRs only (--state open) - Use GITHUB_REPOSITORY for repo instead of hardcoding - Use git diff to detect no-op bumps post-tidy - Read go.mod state via go mod edit -json instead of grep Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
d32b18e to
e5f5427
Compare
babakks
approved these changes
May 13, 2026
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
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.
Summary: Improve bump-go.sh resilience and simplify toolchain handling
Fixes the
bump-go.shscript to tolerate missingtoolchaindirectives ingo.modand simplifies the core logic.Problem
When library maintainers bump their minimum Go version to the latest,
go mod tidyremoves thetoolchainline fromgo.mod(because it becomes redundant). The previous script expected thetoolchaindirective to always be present and used fragilegrep/sedtext manipulation, causing workflow failures.Changes
Replaced manual parsing with
go mod edit:go.modstate viago mod edit -json | jqinstead ofgrepgo mod edit -goand-toolchaininstead ofsedSimplified toolchain logic (addresses review feedback):
goandtoolchaindirectives viago mod editgo mod tidyafterward to normalize — if the toolchain line is redundant, tidy removes it (this is expected Go behavior)Use
git difffor up-to-date detection:git diff --quietAdditional improvements:
go.sumalongsidego.mod(tidy updates both)--state open)$GITHUB_REPOSITORYinstead of hardcodedcli/cliBRANCH_CREATEDflag to avoid errors on early exit