-
Notifications
You must be signed in to change notification settings - Fork 438
Restrict CI build matrix to Linux+MSRV for PRs #4410
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Only run the full build matrix (Linux/Windows/macOS × stable/beta/MSRV) on pushes to main. PR and non-main push builds now only run Linux with the MSRV toolchain (1.75.0), which is the most important gate for catching issues. Co-Authored-By: Claude Opus 4.6 <[email protected]>
|
👋 Thanks for assigning @TheBlueMatt as a reviewer! |
| matrix: | ||
| platform: [ self-hosted, windows-latest, macos-latest ] | ||
| toolchain: [ stable, beta, 1.75.0 ] # 1.75.0 is the MSRV for all crates | ||
| platform: >- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we just move the beta/macos/windows runs to a separate file? That way this stays readable and we can trivially swap it for nightly jobs if we still see backup.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would require duplicating the steps I think? Or extracting the steps in a reusable workflow file perhaps.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, we'd have to copy the steps, but they're basically all just file-download caching.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the current change better. Minimal and non-duplicative. But can do the copy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay. I find it hard to parse but not worth arguing about.
|
👋 The first review has been submitted! Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4410 +/- ##
==========================================
+ Coverage 86.05% 86.06% +0.01%
==========================================
Files 156 156
Lines 103384 103384
Branches 103384 103384
==========================================
+ Hits 88964 88975 +11
+ Misses 11905 11897 -8
+ Partials 2515 2512 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| matrix: | ||
| platform: [ self-hosted, windows-latest, macos-latest ] | ||
| toolchain: [ stable, beta, 1.75.0 ] # 1.75.0 is the MSRV for all crates | ||
| platform: >- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay. I find it hard to parse but not worth arguing about.
Only run the full build matrix (Linux/Windows/macOS × stable/beta/MSRV) on pushes to main. PR and non-main push builds now only run Linux with the MSRV toolchain (1.75.0), which is the most important gate for catching issues.