Skip to content

Add release script wrapping cargo release#8

Open
pthurlow wants to merge 2 commits intomainfrom
fix/release-script
Open

Add release script wrapping cargo release#8
pthurlow wants to merge 2 commits intomainfrom
fix/release-script

Conversation

@pthurlow
Copy link
Collaborator

No description provided.


echo ""
echo "✓ PR created: $PR_URL"
open "$PR_URL"
Copy link
Contributor

Choose a reason for hiding this comment

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

open is macOS-only. With set -euo pipefail, this will cause the script to error-exit on Linux after the PR is already created — leaving the user with a confusing failure despite the PR succeeding.

Use a cross-platform helper:

Suggested change
open "$PR_URL"
if command -v xdg-open &>/dev/null; then
xdg-open "$PR_URL"
elif command -v open &>/dev/null; then
open "$PR_URL"
fi


# step 0: create release branch
echo "→ Creating branch $BRANCH"
git checkout -b "$BRANCH"
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider calling require_clean_tree before checking out the new branch. cargo release will modify Cargo.toml/CHANGELOG.md, so starting from a dirty tree can produce confusing diffs or cause cargo-release to abort mid-run.

Suggested change
git checkout -b "$BRANCH"
require_clean_tree
git checkout -b "$BRANCH"

Copy link
Contributor

@claude claude bot left a comment

Choose a reason for hiding this comment

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

Review

Issues

  • scripts/release.sh:54open "$PR_URL" is macOS-only. With set -euo pipefail, this causes the script to error-exit on Linux after the PR has already been created, leaving the user with a misleading failure. Fix: use a cross-platform xdg-open/open fallback (see inline comment).

  • scripts/release.sh:38prepare is missing a require_clean_tree call before modifying the repo. finish has this guard but prepare doesn't, even though it runs cargo release which edits Cargo.toml and CHANGELOG.md. A dirty tree risks confusing diffs or an unexpected mid-run abort from cargo-release.

Action Required

  1. Fix open "$PR_URL" to work cross-platform (or wrap in a best-effort with || true at minimum to avoid failing after a successful PR creation).
  2. Add require_clean_tree at the top of the prepare branch before git checkout -b.

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