-
Notifications
You must be signed in to change notification settings - Fork 177
[Stellar] Rust environment download add release profile #678
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
Co-authored-by: Eric Lau <ericglau@outlook.com>
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughIntroduces release profiles in the shared workspace Cargo configuration and refactors zip-rust to import workspaceCargo from zip-shared instead of defining it locally. Removes unused imports in zip-rust. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
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 |
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/core/stellar/src/zip-shared.ts (1)
92-106
: Optional: add a TOML comment linking the source of truth.A brief inline comment aids users discovering why these settings exist.
[workspace.dependencies] ${addDependenciesWith(`"${compatibleSorobanVersion}"`, ['soroban-sdk'])}${addDependenciesWith(`"=${contractsVersion}"`, [...libDependencies])} +[#] Profiles recommended by Stellar docs: +[#] https://developers.stellar.org/docs/build/smart-contracts/getting-started/hello-world#release-profile [profile.release] opt-level = "z" overflow-checks = true debug = 0 strip = "symbols" debug-assertions = false panic = "abort" codegen-units = 1 lto = true
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/core/stellar/src/zip-rust.ts
(1 hunks)packages/core/stellar/src/zip-shared.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build (solidity, default)
- GitHub Check: build (stellar, compile)
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (3)
packages/core/stellar/src/zip-shared.ts (2)
103-106
: Verify the profile name: “release-with-logs” vs “release-with-log”.PR text mentions “release-with-log” (singular); here it’s “release-with-logs” (plural). Consistency matters for build commands (cargo build --profile …).
If the docs/use expect singular, apply:
-[profile.release-with-logs] +[profile.release-with-log] inherits = "release" debug-assertions = trueAlso align any references in docs/scripts accordingly.
93-101
: Profile.release settings match Stellar’s recommended configuration Settings align exactly with the current docs (opt-level="z", overflow-checks=true, debug=0, strip="symbols", debug-assertions=false, panic="abort", codegen-units=1, lto=true); no changes needed.packages/core/stellar/src/zip-rust.ts (1)
10-11
: Approve reuse of shared workspaceCargo; no stale definitions found
No local definitions or unintended usages detected outsidezip-shared.ts
.
Fixes #676
Add recommended profile release and profile release-with-log as per documentation