Run the macOS build on pull requests, not only after merge - #942
Conversation
|
Warning Review limit reached
Next review available in: 56 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
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. Comment |
Summary
The macOS build ran only on push, so a pull request could be fully green and still turn
mainred on merge. That is not hypothetical: it happened today. A timing-sensitive test that passes on a fast runner and fails on a slow one went in green and brokemain, and the failure was only visible after the merge that caused it.macOS now runs on pull requests as well. Finding that class of failure before merge costs runner minutes. Finding it after costs a red default branch plus the work to identify which of several merged changes caused it, which in this case was none of them, since the failing test lived in a crate the merge did not touch.
The packaging jobs are deliberately left push-only. Windows, Debian and the reproducibility check verify how artifacts are assembled rather than how the code behaves, so they are much less likely to catch a logic change, and they are the expensive ones to run per pull request. This is a targeted change rather than a blanket one.
Worth stating plainly since it is a recurring cost: macOS runners bill at a higher multiplier than Linux, so this does raise per-pull-request CI spend. Easy to revert by restoring the conditional if that trade is not worth it.
Two related gaps are documented rather than changed, because both are defensible as they stand. Draft pull requests skip the build entirely, which is reasonable for work in progress but means a draft's green checks say nothing; there is a long-lived draft in the Android repository where that is currently misleading. And the Android instrumented suite is likewise gated on draft status. Both are now recorded in the invariants doc so the incompleteness is at least written down.
Test plan
The workflow parses. The matrix is now unconditional, so this pull request itself is the first to exercise it: a
build (macos-latest)job appearing on this PR is the change working.