Conversation
|
|
||
| echo "" | ||
| echo "✓ PR created: $PR_URL" | ||
| open "$PR_URL" |
There was a problem hiding this comment.
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:
| 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" |
There was a problem hiding this comment.
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.
| git checkout -b "$BRANCH" | |
| require_clean_tree | |
| git checkout -b "$BRANCH" |
There was a problem hiding this comment.
Review
Issues
-
scripts/release.sh:54 —
open "$PR_URL"is macOS-only. Withset -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-platformxdg-open/openfallback (see inline comment). -
scripts/release.sh:38 —
prepareis missing arequire_clean_treecall before modifying the repo.finishhas this guard butpreparedoesn't, even though it runscargo releasewhich editsCargo.tomlandCHANGELOG.md. A dirty tree risks confusing diffs or an unexpected mid-run abort from cargo-release.
Action Required
- Fix
open "$PR_URL"to work cross-platform (or wrap in a best-effort with|| trueat minimum to avoid failing after a successful PR creation). - Add
require_clean_treeat the top of thepreparebranch beforegit checkout -b.
No description provided.