ci(release): switch to performance profile for cargo build #2608
Conversation
steebchen
changed the title
WalkthroughOhayo! The pull request introduces updates to the Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
.github/workflows/release.yml (2)
Line range hint
126-134: Fix Windows archive filename inconsistency.Ohayo! There's an issue in the Windows archive handling where the output filename doesn't match the actual file format:
- The archive is created as
.zipbut the output reports.tar.gz- This mismatch could cause issues in downstream processes
- echo "file_name=dojo_${VERSION_NAME}_${PLATFORM_NAME}_${ARCH}.tar.gz" >> $GITHUB_OUTPUT + echo "file_name=dojo_${VERSION_NAME}_${PLATFORM_NAME}_${ARCH}.zip" >> $GITHUB_OUTPUT
Line range hint
107-134: Add binary size verification step.Ohayo sensei! Since this PR focuses on binary size optimization, we should add a step to verify and log the size improvements.
Add this step after the build:
- name: Verify binary sizes run: | echo "Binary sizes:" if [ "$PLATFORM_NAME" == "win32" ]; then ls -l ./target/${TARGET}/release/*.exe else ls -l ./target/${TARGET}/release/{katana,sozo,torii,dojo-language-server} fiThis will help track size optimization progress across releases.
Cargo.toml (1)
Ohayo! Consider updating slot dependency to latest stable version v0.21.0
The slot dependency is currently pinned to git revision
1298a30, but there are stable releases available. The latest version is v0.21.0. Using a stable version instead of a git revision would:
- Provide better dependency tracking
- Make updates more predictable
- Follow Rust ecosystem best practices
Replace:
slot = { git = "https://github.com/cartridge-gg/slot", rev = "1298a30" }with:
slot = { git = "https://github.com/cartridge-gg/slot", tag = "v0.21.0" }🔗 Analysis chain
Line range hint
284-284: Consider pinning slot dependency to a stable version.The slot integration is currently pinned to a specific git revision (
1298a30). While this works, it might be better to:
- Wait for a stable release and use version numbers
- Document why this specific revision was chosen
Let's check if there are any stable releases available:
Note: The previous script failed to fetch GitHub releases information. Let's first verify the actual content of Cargo.toml to understand the context better, as the review comment snippet was empty. This will help us determine our next steps for verification.
Note: Since the GitHub releases API call failed earlier, let's try checking for tags instead to see if there are any stable versions available for the slot dependency.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check slot repository for releases gh api repos/cartridge-gg/slot/releases --jq '.[].tag_name' || echo "No releases found"Length of output: 163
Script:
#!/bin/bash # Let's check the actual content of Cargo.toml to verify the slot dependency cat Cargo.tomlLength of output: 8700
Script:
#!/bin/bash # Let's check if there are any tags in the slot repository gh api repos/cartridge-gg/slot/tags --jq '.[].name' || echo "No tags found"Length of output: 275
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
.github/workflows/release.yml(1 hunks)Cargo.toml(1 hunks)
🔇 Additional comments (3)
.github/workflows/release.yml (1)
107-107: Ohayo sensei! Verify the performance profile configuration.
The change from --release to --profile performance aligns with the size optimization goal. However, let's ensure the profile is properly configured.
✅ Verification successful
Ohayo sensei! The performance profile is properly configured! ٩(◕‿◕｡)۶
The performance profile in Cargo.toml is correctly set up with opt-level = "z" for maximum size optimization, which perfectly aligns with the build command change in the workflow.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify performance profile configuration in Cargo.toml
rg -U '\[profile\.performance\][\s\S]*?opt-level.*?=.*?"z"' || echo "Performance profile with opt-level 'z' not found"
Length of output: 150
Cargo.toml (2)
Line range hint 1-2: Workspace resolver configuration is appropriate.
Using resolver = "2" is the recommended choice for modern Rust workspaces, providing better dependency resolution.
Line range hint 61-65: Ohayo sensei! Performance profile configuration looks solid!
The optimization settings are well-configured for maximum binary size reduction:
opt-level = "z"prioritizes size over speedlto = "fat"enables aggressive cross-module optimizationcodegen-units = 1maximizes optimization potentialincremental = falseis appropriate for release builds
Let's verify the impact on binary size:
#!/bin/bash
# Description: Compare binary sizes before and after optimization
# Note: This requires the PR branch to be checked out
# Get the current binary sizes
echo "Current binary sizes:"
fd -t f -e exe . target/performance/ -x ls -lh {} \; 2>/dev/null || echo "No binaries found yet"
Updated the GitHub Actions release workflow to use the 'performance' profile instead of 'release' for building binaries.
steebchen
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2608 +/- ##
==========================================
- Coverage 55.11% 54.92% -0.19%
==========================================
Files 385 393 +8
Lines 47775 48328 +553
==========================================
+ Hits 26332 26546 +214
- Misses 21443 21782 +339 ☔ View full report in Codecov by Sentry. |
Description
Updated the GitHub Actions release workflow to use the 'performance'
profile instead of 'release' for building binaries.
Tests
Added to documentation?
Checklist
scripts/prettier.sh,scripts/rust_fmt.sh,scripts/cairo_fmt.sh)scripts/clippy.sh,scripts/docs.sh)Summary by CodeRabbit