chore(releasing): publish to crates.io after prep PR is merged#1783
chore(releasing): publish to crates.io after prep PR is merged#1783thomasqueirozb wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 82406c4d6b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if state.trim() == "MERGED" { | ||
| println!("PR merged."); | ||
| break; | ||
| } | ||
| println!(" state: {}. Checking again in 15s...", state.trim()); | ||
| std::thread::sleep(std::time::Duration::from_secs(15)); |
There was a problem hiding this comment.
Abort when the prep PR is closed
In the release flow where the prep PR is closed instead of merged (for example reviewers reject it or a maintainer recreates the PR), this loop never exits because it treats every non-MERGED gh pr view --json state result as still pending; the GitHub CLI exposes closed PRs through this same state field. That leaves the release command polling forever rather than failing before tagging/publishing can be retried or corrected.
Useful? React with 👍 / 👎.
|
|
||
| println!("Switching to main..."); | ||
| run("git", &["checkout", "main"], &root)?; | ||
| run("git", &["pull", "origin", "main"], &root)?; |
There was a problem hiding this comment.
Pin the tag to the merged release PR
When another PR lands on main after the release prep PR is merged but before this command runs, git pull origin main advances to that newer head, so the following tag and cargo publish operate on commits that were not part of the reviewed release PR. This can publish unintended code/changelog content under the release version; the script should tag/publish the prep PR's merge commit rather than whatever origin/main happens to be when polling notices the merge.
Useful? React with 👍 / 👎.
| run( | ||
| "git", | ||
| &["tag", "-a", &tag, "-m", &format!("Release {new_version}")], | ||
| &root, | ||
| )?; |
There was a problem hiding this comment.
Create the release tag only after publish succeeds
If cargo publish fails after this tag is created (for example because of a crates.io/auth/network error or because the merged package still needs a fix), the command exits with an unpublished local v{new_version} tag pointing at the failed commit. A retry of the release flow then fails at tag creation or requires manual tag cleanup, and if the fix is made in a follow-up commit the stale tag no longer matches the releasable commit; keep the tag creation after a successful publish or clean it up on failure.
Useful? React with 👍 / 👎.
| break; | ||
| } | ||
| println!(" state: {}. Checking again in 15s...", state.trim()); | ||
| std::thread::sleep(std::time::Duration::from_secs(15)); |
There was a problem hiding this comment.
💭 Can we avoid polling? I am thinking if we can use the tag as a trigger for a publish workflow.
Summary
The release script was publishing to crates.io and tagging before creating the prep PR, meaning releases landed on a non-main branch. The script now creates the PR first, polls until it is merged, then tags on main, checks out the tag, publishes, and pushes the tag.
Also updates the release issue template to reference
cargo run -p releaseinstead of the removed manual scripts.Change Type
Is this a breaking change?
How did you test this PR?
NA - not able to test easily until next release
Does this PR include user facing changes?
our guidelines.
Checklist
run
dd-rust-license-tool writeand commit the changes. More details here.References
NA