Conversation
A build or deploy step that is a shell invocation rather than a JavaScript config file cannot import this package at all, so it has had no way to reach resolveBuildIdentity/predictNextVersion/resolvePredictedIdentity. `wrangler deploy --var RELEASE_VERSION:...` is the concrete case: the value has to exist as a shell variable before wrangler runs, and no config file is involved anywhere in that path. The command reuses those three functions rather than reimplementing any of them, and prints a single DisplayIdentity under exactly the field names the type itself declares. Nothing here is named for a consuming repo, and there is no `predicted` boolean alongside `kind: "predicted"` -- a second field asserting the same fact is one more thing that can disagree with the first. Prediction sits behind an explicit --predict rather than happening by default: it needs the optional @semantic-release/commit-analyzer peer dependency and reads every commit since the last tag, neither of which a caller that only wants the release-or-commit answer should pay for silently. Release rules must come from the caller for the same reason the library takes them as an argument -- a repo's commit-type convention is real configuration this package has no authority to default. --format env is deliberate extra surface over "print JSON, document a jq one-liner". Appending to $GITHUB_ENV is the realistic use in both of the cases that prompted this, and the jq alternative would push a dependency and a quoting-sensitive shell expression into every consumer's workflow to produce output the command can simply print. Its names stay generic (an upper-cased field behind --prefix), so mapping onto a repo's own names remains that repo's job. A value carrying a line break is refused rather than emitted, since it would otherwise swallow or inject $GITHUB_ENV entries unnoticed. commander is the package's first runtime dependency, and is bundled into dist/cli.js alone: importing the library never loads it.
Covers usage, every flag, both output formats, and the release-rules requirement behind --predict. The contrast is the part worth stating explicitly, because getting it wrong is the likely failure: a next.config.ts or vite.config.ts is itself JavaScript and should import the functions directly, which is simpler and better typed than shelling out and parsing JSON back. The command is for steps with genuinely nothing to import from -- `wrangler deploy --var`, `docker build --build-arg`, a plain sh deploy script.
commander@15 declares engines.node >=22.12.0, while this package declares >=20. Installing it under a package manager that enforces engine ranges would fail on any Node 20 runtime the package still claims to support, and shipping that contradiction is worse than being a major behind on a dependency whose API surface here is a handful of long-stable methods. commander@14 declares >=20, exactly matching. Raise it when this package's own Node floor rises, not before.
commit-analyzer narrates through printf-style calls, logger.log("Analyzing
commit: %s", message) among them. Joining the arguments with a space left the
literal %s sitting in the output next to the value that was meant to replace
it. node:util's format applies the substitution the caller intended, and falls
back to inspecting any argument with no placeholder to fill.
Mearman
marked this pull request as ready for review
September 8, 2026 14:55
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
🎉 This PR is included in version 2.1.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
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.
Adds a
build-identitybin so build and deploy steps that are a shell invocation, rather than a JavaScript config file, can reach this package at all. The case it was built for iswrangler deploy --var RELEASE_VERSION:...: the value has to exist as a shell variable before wrangler runs, and there is no config file anywhere in that path to import from.The command reuses
resolveBuildIdentity,predictNextVersionandresolvePredictedIdentitydirectly rather than reimplementing any of their logic, and prints a singleDisplayIdentityunder exactly the field names that type declares. No consumer-specific naming, and nopredictedboolean alongsidekind: "predicted".Three decisions worth flagging for review:
--predictis an explicit opt-in. It needs the optional@semantic-release/commit-analyzerpeer dependency and reads every commit since the last tag; a caller that only wants the release-or-commit answer should not pay for either silently. Release rules must come from the caller for the same reason the library takes them as an argument.--format envexists rather than "print JSON, document ajqone-liner". Appending to$GITHUB_ENVis the realistic use in both motivating cases, and thejqroute would push a dependency and a quoting-sensitive shell expression into every consumer's workflow to produce output the command can just print. Names stay generic (an upper-cased field name behind--prefix), so mapping onto a repo's own names stays that repo's job. A value carrying a line break is refused rather than emitted, since it would otherwise swallow or inject$GITHUB_ENVentries unnoticed.commanderis the package's first runtime dependency. It is bundled intodist/cli.jsalone, so importing the library never loads it. The alternative was hand-rolling argument parsing to keep the package dependency-free, which buys nothing but a worse--helpand worse errors.The README now also states the contrast explicitly, since getting it backwards is the likely failure: a
next.config.tsorvite.config.tsis itself JavaScript and should import the functions directly, which is simpler and better typed than shelling out and parsing JSON back.Tests: unit coverage for argument parsing, output formatting and release-rule validation, plus an end-to-end suite that builds the real bin and runs it as a subprocess against disposable git fixture repositories.