Skip to content

ci: pass matrix target to release upload and gate on release creation#5

Open
pnodet wants to merge 1 commit into
mainfrom
ci/fix-release-target-matrix
Open

ci: pass matrix target to release upload and gate on release creation#5
pnodet wants to merge 1 commit into
mainfrom
ci/fix-release-target-matrix

Conversation

@pnodet

@pnodet pnodet commented Jul 3, 2026

Copy link
Copy Markdown
Member

Summary

Fixes three issues in the upload-bin job of the release workflow:

  1. Pass target: ${{ matrix.target }} to taiki-e/upload-rust-binary-action. The job defined a five-target matrix (aarch64/x86_64 for Linux and macOS, plus universal-apple-darwin) but never told the action which target to build. Without a target input the action builds the host triple, so the aarch64-linux entry actually shipped an x86_64 binary, universal-apple-darwin was not built as a universal binary, and matrix entries collided on artifact names.

  2. Add needs: create-release to upload-bin. Uploads could previously race the release-creation job and fail if the GitHub release did not exist yet.

  3. Remove features: ${{ matrix.features || '' }}. No matrix entry defines a features key; the line was copy-paste cruft.

Everything else (checkout, token, tar/zip settings, matrix entries) is unchanged.

🤖 Generated with Claude Code

The upload job never passed target to the action, so every matrix
entry built the host triple and cross-target artifacts were wrong.
Also make uploads wait for the release to exist and drop the unused
matrix.features reference.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your trial has ended. Reactivate Greptile to resume code reviews.

@coderabbitai

coderabbitai Bot commented Jul 3, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@pnodet, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 59 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ffca3078-b820-4fb6-ab3f-b03bd1d05b45

📥 Commits

Reviewing files that changed from the base of the PR and between e20b82b and 138adbd.

📒 Files selected for processing (1)
  • .github/workflows/release.yml
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ci/fix-release-target-matrix

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@pnodet

pnodet commented Jul 4, 2026

Copy link
Copy Markdown
Member Author

Thermo-nuclear code quality review

Strict review of the diff vs main (touches only .github/workflows/release.yml, matching the stated scope — no scope creep).

Correctness of the fix

  1. target: ${{ matrix.target }} — correct and necessary. Before this PR the matrix was decorative: upload-rust-binary-action built for the host's default triple, so both Linux entries produced x86_64 binaries and the three Darwin entries all produced whatever the runner's native arch was, with the aarch64/universal artifacts being mislabeled duplicates at best and upload-name collisions at worst. Passing the matrix target makes every entry build the triple it claims to.
  2. Cross-compilation for aarch64-unknown-linux-gnu on ubuntu-latest — verified OK. Per the action's README, when target differs from the host it uses cross by default (installing it via cargo install cross --locked if absent), so no extra toolchain setup step is required. Docker is available on ubuntu-latest, which is all cross needs.
  3. universal-apple-darwin — verified OK. The README explicitly supports it ("Universal macOS binary is supported as universal-apple-darwin"); the action builds both Darwin triples and lipo-merges them itself.
  4. needs: create-release — correct gating. The upload step attaches assets to the tag's release; without the dependency there is a race where an upload job wins against create-gh-release-action and fails. This also serializes correctly on re-runs.
  5. Removing features: ${{ matrix.features || '' }} — clean dead-code deletion. No matrix entry ever defined features, so the expression always evaluated to the empty string. Deleting it removes a phantom knob rather than preserving incidental complexity — exactly the right instinct.

YAML validated (yaml.safe_load passes).

Non-blocking observations (not fix-worthy in this PR)

  • The Darwin story ships three artifacts (x86_64, aarch64, universal). That is a common taiki-e pattern and defensible, but if download-count simplicity ever matters, universal-apple-darwin alone would cover all Macs and delete two matrix rows.
  • zip: all + tar: unix doubles every unix artifact (tar.gz and zip per target). Pre-existing behavior, out of scope here.

Verdict

Approve. The diff is minimal, structurally clean, deletes dead configuration instead of accreting it, and fixes a real correctness bug in the release matrix. Meets the approval bar: no structural regression, no spaghetti growth, no missed simplification within the PR's scope.

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