Skip to content

fix(release): read the version after checking out the base - #82

Merged
anoop-narang merged 3 commits into
mainfrom
fix/release-prepare-version-order
Sep 10, 2026
Merged

fix(release): read the version after checking out the base#82
anoop-narang merged 3 commits into
mainfrom
fix/release-prepare-version-order

Conversation

@anoop-narang

Copy link
Copy Markdown
Contributor

The bug

scripts/release.sh cmd_prepare reads the current version before it switches to the base branch:

current="$(get_version)"          # <- reads whatever branch you are on
new="$(bump_version "$bump" "$current")"
...
base="$(default_branch)"
git checkout "$base"              # <- only now on the release base
set_version "$new"

So the bump is computed from the branch the script happened to be invoked on, not from the branch being released.

How it shows up

Hit for real while releasing another repo in this family. On a stale branch at 0.10.0, prepare minor computed 0.11.0, then checked out main at 0.16.0:

Error: Cannot downgrade from 0.16.0 to 0.11.0

That failure was luck. It only errored because the version tool refuses downgrades and the branch happened to be behind. From a branch that is ahead of the base, the computed version is higher, nothing refuses it, and a release is cut under the wrong number with no error at all.

The fix

Move the version read below the checkout, so it reads the base branch. hotdata-ibis already orders it this way; this brings the rest of the family in line. Pure block move plus a comment — no behaviour change when the script is run from an already-current base branch, which is the documented path.

Scope

The same defect is in seven repos sharing this script. Companion PRs are open in each.

`cmd_prepare` read the current version before switching to the base
branch, so the bump was computed from whatever branch the script
happened to be on. Running it from any other branch cuts a release
numbered off unrelated history.

`scripts/release.sh` in hotdata-ibis already has this ordering; this
brings the rest in line.
@anoop-narang
anoop-narang requested a review from a team as a code owner September 10, 2026 14:27
@anoop-narang
anoop-narang requested review from eddietejeda and removed request for a team September 10, 2026 14:27
claude[bot]
claude Bot previously approved these changes Sep 10, 2026

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Block move is correct. The order now matches cmd_publish in scripts/release.sh, which reads the version after checking out the base.

Moving the version read below the checkout also moved the bump-kind
validation, which lives inside bump_version. A typo then switched the
caller to the base branch before failing.

Check the argument next to the existing emptiness check, using the same
pattern the explicit-version branch already accepts, so an argument
error exits without side effects as it did before.
@anoop-narang

Copy link
Copy Markdown
Contributor Author

Follow-up applied here too, for consistency across the family.

Reviewers on four of the sibling PRs noticed that moving the version read below the checkout also moved the bump-kind validation, since it lives inside bump_version — so a typo like prepare pathc would switch you to the base branch before failing. That was a regression this change introduced.

$bump is now validated against ^(patch|minor|major|[0-9]+\.[0-9]+\.[0-9]+)$ next to the existing emptiness check, so an argument error exits with no side effects. Same pattern the explicit-version branch already accepts, so nothing valid is newly rejected.

claude[bot]
claude Bot previously approved these changes Sep 10, 2026

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved. The version read now happens after the base checkout, so the bump derives from the released branch. The early bump-kind guard accepts exactly the documented set, so no valid input becomes rejected.

The explicit-version regex appeared in both the argument check and the
branch that takes the argument verbatim, so a change to the accepted
format had to be made twice. Hoist it to VERSION_RE, alongside
BUMP_KIND_RE.

Also narrow the comment above the check: an unchanged version and a
pre-release suffix are found from the base branch version, so those
necessarily fail after the checkout and the comment should not imply
otherwise.
@anoop-narang

Copy link
Copy Markdown
Contributor Author

Two follow-ups from review applied across all seven repos in this family, so they stay identical:

  1. VERSION_RE / BUMP_KIND_RE hoisted to one definition each, since the explicit-version pattern was otherwise written twice and could drift.
  2. The comment above the argument check narrowed — an unchanged version and a pre-release suffix are detected from the base branch's version, so those necessarily fail after the checkout and the comment should not imply they don't.

Accept/reject behaviour is unchanged: patch/minor/major/1.2.3 accepted; pathc, empty, 1.2, v1.2.3, 1.2.3-rc1 rejected.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved. The argument check now runs before any branch switch, and the shared VERSION_RE keeps the check and the verbatim-assignment branch in step. The unquoted variable on the right of =~ is the correct form, so both patterns still apply as regexes.

@anoop-narang
anoop-narang merged commit 646686e into main Sep 10, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant