fix: move cdylib to separate float-wasm crate#174
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughCreates a new WASM wrapper crate (rain-math-float-wasm) that builds as a cdylib and re-exports the core rain-math-float API; the core crate's Cargo.toml is changed to produce only rlib artifacts. The build script reference to the emitted wasm filename was updated. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/float/Cargo.toml`:
- Around line 8-9: Update the build script to point to the new WASM artifact
name produced by the changed crate-type: in scripts/build.js locate the
references to "rain_math_float.wasm" and replace them with
"rain_math_float_wasm.wasm" so the paths passed to wasm-bindgen (e.g.,
"./target/wasm32-unknown-unknown/release/...") match the actual output from the
rain-math-float-wasm crate; ensure both occurrences currently referencing
"rain_math_float.wasm" are updated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1eae2097-af05-4d4b-a063-e7ecc180fc62
📒 Files selected for processing (3)
crates/float-wasm/Cargo.tomlcrates/float-wasm/src/lib.rscrates/float/Cargo.toml
b26bc6f to
f9f9d96
Compare
|
@coderabbitai assess this PR size classification for the totality of the PR with the following criterias and report it in your comment: S/M/L PR Classification Guidelines:This guide helps classify merged pull requests by effort and complexity rather than just line count. The goal is to assess the difficulty and scope of changes after they have been completed. Small (S)Characteristics:
Review Effort: Would have taken 5-10 minutes Examples:
Medium (M)Characteristics:
Review Effort: Would have taken 15-30 minutes Examples:
Large (L)Characteristics:
Review Effort: Would have taken 45+ minutes Examples:
Additional Factors to ConsiderWhen deciding between sizes, also consider:
Notes:
|
|
Tip For best results, initiate chat on the files or code changes. SIZE=S |
Problem
Having
crate-type = ["rlib", "cdylib"]causes Cargo to produce output artifacts with colliding filenames. On Linux, this results inalloy_primitivesbeing compiled as two separate crate instances, causing type mismatches for any downstream crate:See: rust-lang/cargo#6313
Fix
cdylibfromcrate-typeincrates/float/Cargo.toml, keeping onlyrlibcrates/float-wasm/crate withcrate-type = ["cdylib"]that simply re-exportsrain-math-floatWASM consumers should depend on
rain-math-float-wasminstead ofrain-math-floatdirectly.Summary by CodeRabbit
New Features
Refactor