-
-
Notifications
You must be signed in to change notification settings - Fork 738
feat: apply binary size check for mac and windows #12200
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
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for rspack canceled.
|
10315fd to
2d90dbc
Compare
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.
Pull Request Overview
This PR extends binary size monitoring from Linux-only to include macOS and Windows platforms, enabling size tracking for the binaries that most users actually use.
Key Changes:
- Added matrix strategy to run binary size checks on Linux (x86_64-unknown-linux-gnu), macOS (aarch64-apple-darwin), and Windows (x86_64-pc-windows-msvc) platforms
- Refactored binary-limit action to support multiple platforms with platform-specific binary path mappings
- Implemented a two-stage workflow: individual platform checks that upload results, followed by a gather-results job that combines and posts a unified report
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
.github/workflows/size-limit.yml |
Converted from single-platform to matrix-based execution; added artifact collection and result aggregation |
.github/actions/binary-limit/binary-limit-script.js |
Added platform parameter support, binary path mappings for all platforms, and multi-platform report formatting |
.github/actions/binary-limit/action.yml |
Added platform input parameter and result output for multi-platform support |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| with: | ||
| script: | | ||
| const limit = parseInt("${{ inputs.size-threshold }}") || 51200; | ||
| const platform = inputs.platform; |
Copilot
AI
Nov 14, 2025
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.
The platform input should be accessed using GitHub Actions expression syntax. Line 30 attempts to access inputs.platform directly in JavaScript, but inputs is not available in the script context. It should be "${{ inputs.platform }}" to match the pattern used on line 29 for size-threshold.
| const platform = inputs.platform; | |
| const platform = "${{ inputs.platform }}"; |
| run: | | ||
| rustup target add x86_64-unknown-linux-gnu | ||
| RUST_TARGET=x86_64-unknown-linux-gnu pnpm build:binding:release | ||
| rustup target add ${{ matrix.target }} |
Copilot
AI
Nov 14, 2025
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.
Redundant rustup target add command. This target was already added in the "Setup Rust Target" step at line 47. The duplicate command on line 59 should be removed.
| rustup target add ${{ matrix.target }} |
| - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5 | ||
|
|
||
| - name: Download size reports | ||
| uses: actions/[email protected] |
Copilot
AI
Nov 14, 2025
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.
Inconsistent action version format. The action version should be @v4.1.7 (with v prefix) to match the convention used elsewhere in the repository (e.g., line 21 in .github/actions/artifact/download/action.yml).
| uses: actions/download-artifact@4.1.7 | |
| uses: actions/download-artifact@v4.1.7 |
CodSpeed Performance ReportMerging #12200 will not alter performanceComparing Summary
|
2d90dbc to
b53d351
Compare
Summary
Current only binary size of linux is checked, but most users use mac and windows, should watch size changes of them as well
Related links
Checklist