Skip to content

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

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

fix(release): read the version after checking out the base#30
anoop-narang merged 3 commits into
masterfrom
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 shefeek-jinnah and removed request for a team September 10, 2026 14:27
Comment thread scripts/release.sh
claude[bot]
claude Bot previously approved these changes Sep 10, 2026
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.
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.

Prior super nit is resolved: the bump kind is now validated at scripts/release.sh:106, before the checkout at scripts/release.sh:115. The version read now happens on the base branch, which fixes the wrong-base bump. No new findings.

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.

Prior feedback is addressed. The bump kind check now runs before the checkout, and the shared version pattern is used unquoted on the right side of =~, which keeps regex matching intact.

@anoop-narang
anoop-narang merged commit ed45527 into master 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