refactor(scripts): turn bump-version into an explicit release flow - #294
Merged
antobinary merged 10 commits intoSep 17, 2026
Merged
antobinary merged 10 commits into
antobinary merged 10 commits into
Conversation
The release script derived the next version in bash, with `cut -d. -f3` followed by
`$((n + 1))`. That works for a stable version, but on a pre-release it reads the wrong
field: `cut -d. -f3` on "1.0.0-beta.1" yields "0-beta", and `$(("0-beta" + 1))` is
evaluated as `0 - beta + 1` with `beta` an unset variable, so it quietly returns 1. The
trailing-number substitution then rewrites the pre-release counter instead of the patch:
1.0.0-beta.1 stayed on 1.0.0-beta.1, and 1.0.0-rc.2 walked backwards to 1.0.0-rc.1. No
error was raised in either case.
Move the arithmetic to Node, which the script already depended on to read package.json:
- nextVersion moves the patch of a stable version and the counter of a pre-release, and
refuses a pre-release that carries no counter to move.
- distTagFor derives the npm dist-tag from the version, so a pre-release is published
under its own channel rather than taking over "latest".
- compareVersions orders two versions by the semantic versioning rules, so a release that
would not move the package forward can be refused before anything is published.
The module doubles as a command line entry point, so the shell gets a readable message and
a non-zero exit code instead of a stack trace when a version is rejected.
bump-version.sh did four things under a name that announces one: it bumped the version, published to npm, pointed the 23 samples at the new version, and committed, tagged and pushed. The name also hid the order, which is not the one the name suggests: the package reaches npm before the git tag exists. Rename it to publish-version.sh and split the two steps that are worth running on their own: - publish-to-npm.sh <VERSION> publishes under the dist-tag the version implies. This replaces the `--tag tmp` / `dist-tag add latest` / `dist-tag rm tmp` dance, which forced "latest" onto every publish. With two maintained lines that is a hazard: a maintenance release on the older line takes "latest" away from the newer one, and a pre-release would take it away from every stable release. - publish-git-tag.sh <VERSION> commits the version files, tags the commit and pushes. publish-version.sh keeps orchestrating the whole release. Called without an argument it behaves as before, releasing the version that follows the current one. Called with a version it releases exactly that one, which is how a new pre-release channel is opened. The version is now written with `npm version --no-git-tag-version` instead of a `sed` on the raw JSON, so package.json and package-lock.json stay in agreement. All three scripts follow the convention publish-to-project-folder.sh already established in this directory: `set -e`, paths resolved from the script location, a usage message, and explicit guards. They refuse a version that is not a version, a version that does not move the package forward, a git tag that already exists and a dirty working tree, and they all take --dry-run, so a release can be rehearsed before it leaves the machine.
publish-version.sh validated an explicitly requested version by asking for the version that follows it. That is the wrong question: a pre-release such as 1.0.0-beta carries no counter to move, so nextVersion refuses it and tells the caller to pass the version explicitly. Passing it explicitly hit the same check and got the same message, which left no way to release that version at all. Validation now asks whether the version is well formed, through a validate command backed by parseVersion. nextVersion keeps its refusal, which is correct where it is used: deriving a successor automatically. The tests pin the distinction, since every function other than nextVersion has to accept a counterless pre-release.
The SDK had no written release procedure: the command to run, the version it would pick and the dist-tag it would publish under all lived in the scripts themselves. A maintainer had to read bash to release. Add a "Releasing a New Version" section covering publish-version.sh with and without an explicit version, the dist-tag derived from the version, the --dry-run rehearsal and the four guards that refuse a release, each with the message the script actually prints. It closes on publish-to-npm.sh and publish-git-tag.sh, which are runnable on their own, and on the shared version arithmetic in scripts/lib/version.js.
The parsing, the increment and the ordering in scripts/lib/version.js were written by hand, and pre-release precedence is the part of semantic versioning that is easiest to get subtly wrong. They now come from the semver library, added as a devDependency: it is used only by the publish scripts and is never bundled into dist, so nothing changes for a package consumer. Two rules stay local because they are not semver's to decide: - a pre-release carrying no numeric counter is refused instead of being started at ".0", which is what semver.inc does to "1.0.0-beta" - the npm dist-tag is derived from the pre-release channel, a concept the library does not have parseVersion is also stricter than semver on purpose. semver normalises what it accepts, so "v1.0.0" and "1.0.0+build.5" both come back as "1.0.0"; comparing that result against the input preserves the refusal this file already documented. The 16 unit tests covering the module are unchanged and still pass, which is what makes the swap safe to make. scripts/** gains an eslint override: the release tooling is CommonJS invoked by the publish scripts, so requiring a development dependency there is expected rather than a finding.
nextVersion parsed the version twice: once through parseVersion, once through semver.prerelease. Only the second reading is needed, because it is the typed one - a channel stays a string, a counter comes back a number - and the example in the error message is the version itself with a counter appended, not the version rebuilt part by part. publish-version.sh asked the library to validate the requested version and then, two lines later, asked it for the dist-tag, which validates the same way and fails the same way. Resolving the dist-tag first makes the explicit validate redundant, and it also has to come first: the comparison that follows runs in a command substitution, where a failure would have been reported as a version that does not move forward. The guards that stay are the ones semver does not make: a pre-release with no counter, which semver would happily start at .0, a version that only differs from a valid one by normalisation, such as v1.0.0, and a pre-release channel npm would refuse as a dist-tag.
The release scripts explained themselves line by line - that "set -e" exits on failure, that a cd changes directory, that an echo prints a message - which buries the handful of comments that carry something the code does not: why the strict comparison against semver.valid, why the counter is refused before semver.inc, why publishing is what proves the tarball assembles. What the scripts were missing is where their steps begin and end, so publish-version.sh now marks its three sections - publishing to npm, pointing the samples at the new version, and committing, tagging and pushing to github - in the real run and in the dry run alike, with the same names used in the two scripts it delegates to.
The publish scripts ran from whatever branch and clone they were invoked in, so a release could leave from a stale fork or from a branch never meant to carry one. A new guard, called before anything else by all three scripts, refuses unless the current branch tracks one of the release branches declared in scripts/lib/release-branches.json on the main repository and points at the same commit as the remote tip. The comparisons are made on the remote side, because a fork can name the same branch differently and a tracking remote is not necessarily the main repository. Under --dry-run the guard only reports what a real run would refuse, so a dry run still works from any branch or clone.
The three release scripts shared the branch guard, the dist-tag resolution and the dry-run idiom, and the split made every safety check run three times per release. publish-version.sh is now the single entry point, with the npm stage and the git stage gated by PUBLISH_TO_NPMJS and PUBLISH_TO_GITHUB. The toggles are environment overrides because flipping a stage must not require editing the file: an edited script dirties the tree and the clean-tree check would refuse the release. PUBLISH_TO_NPMJS=false finishes a release whose npm publish already succeeded: the version is the one package.json already holds, and the clean-tree and not-higher checks are skipped because the uncommitted version files are exactly what the git stage commits. The samples loop lives in the git stage because that is the stage that commits the files the loop produces. The consolidation also carries the review fixes in their folded form: npm ci instead of npm install before the build so the tarball builds from the pinned dependency tree, rm -rf dist before npm publish so stray output of an old build cannot ship, the sample directory quoted in the loop, and the branch name passed to node as data instead of interpolated into source, which closes a code injection in the release-branch check.
The standalone step scripts are gone, so the README now documents the two stage toggles with their invocation form, and marks the not-higher and uncommitted-changes guards as npm-stage guards that the git-only re-run skips by design.
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
2 participants
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.
What does this PR do?
scripts/bump-version.shtoscripts/publish-version.sh. With no argument it behaves exactly as before: release the version that follows the current one../scripts/publish-version.sh 1.0.0-beta.1releases exactly that version, and the npm dist-tag now follows the version (latestfor a stable one,beta/rc/alpha/nextfor a pre-release) instead of being forced tolatest.scripts/publish-version.sh, with the npm stage and the git stage toggled by the environment variablesPUBLISH_TO_NPMJSandPUBLISH_TO_GITHUB(both defaulttrue).PUBLISH_TO_NPMJS=false ./scripts/publish-version.shre-runs only the commit, tag and push after a publish that already landed.scripts/lib/release-branches.json.npm cibefore the build, so the tarball builds from exactly the locked tree the release commits, and deletesdist/beforenpm publish: git ignores it, so stray output of an old build would otherwise ship silently.scripts/lib/version.js, built on thesemverlibrary (new devDependency) and covered by 16 unit tests.semveralready enforces (e.g. avalidatecall inpublish-version.shwhose check the dist-tag resolution repeats right after, a double parse innextVersion). Guards that enforce behaviorsemverdoes not guarantee (pre-release counter beforeinc, strict no-v-prefix, dist-tag format) are kept, each annotated with why it exists. Comments that repeated the identifier are trimmed; section demarcations (npm publish, tag+commit, samples) are added topublish-version.sh.Closes Issue(s)
None, this is not tracked by an issue.
Motivation
The old script forced
lateston every publish, so a maintenance release on an older line would quietly takelatestaway from the newer one, and a pre-release would take it away from every stable release.It also got pre-release arithmetic wrong, silently:
cut -d. -f3on1.0.0-beta.1returns0-beta, so the increment lands on the pre-release counter instead of the patch.1.0.0-beta.1recomputes to itself (npm answers 403 after the build has already run) and1.0.0-rc.2walks backwards to1.0.0-rc.1. Pre-releases cannot be published at all today.More
How to test, neither command publishes, commits, tags or pushes:
Lint,
npx tsc,npm ci, the build and the full 23-sample validation job were all run locally on this change and pass.npm run test:unitpasses 48 of 48 tests (32 already in the repository, 16 new). No version is released by this PR:package.jsonstays at0.1.26.The arithmetic and the ordering come from
semver; two rules stay local because they are not semver's to decide: a pre-release with no numeric counter is refused instead of silently starting one at.0, and the npm dist-tag is derived from the pre-release channel. Requiring the library fromscripts/**needs an eslint override there, since that code is CommonJS release tooling and is never bundled intodist.The README documents the flow: a new "Releasing a New Version" section, between "Testing SDK" and "API", covers both invocations, the stage toggles, the dist-tag derived from the version,
--dry-runand the five guards, with the output each one actually prints. The "not higher" and "uncommitted changes" guards belong to the npm stage, so the git-only re-run skips them by design. The branch guard is the only one that needs the network, to read the remote tip and confirm the branch is in sync; under--dry-runit reports what a real run would refuse and continues.Known limitations:
The same script exists on
v0.0.xandv0.1.xwith the same defect; porting it there is a separate decision, out of scope here.The six pre-existing scripts under
scripts/(build_all.sh,publish-to-samples.sh, etc.) still carry the same verbose-comment pattern; cleaning them is a separate, focused PR.Added/updated documentation