-
Notifications
You must be signed in to change notification settings - Fork 1
Update getrandom version in text_fixtures crate
#424
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
WalkthroughThe getrandom dependency version is updated from "0" to "0.2" in the WebAssembly target configuration of the test fixtures manifest file. Features remain unchanged. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
0xgleb
left a 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.
not sure if this changes anything given that there is no Cargo.lock diff
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
crates/test_fixtures/Cargo.toml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-09T07:46:10.639Z
Learnt from: 0xgleb
Repo: rainlanguage/rain.interpreter PR: 333
File: Cargo.toml:11-11
Timestamp: 2025-06-09T07:46:10.639Z
Learning: When analyzing dependency upgrades in Rust projects, always search for the specific programming language and context (e.g., "Rust alloy crate" vs "Alloy framework") to avoid confusion between different libraries with the same name. The alloy crate in Rust is for Ethereum development, not mobile app development.
Applied to files:
crates/test_fixtures/Cargo.toml
⏰ 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). (10)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-static)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-artifacts)
- GitHub Check: rainix (macos-latest, rainix-rs-artifacts)
- GitHub Check: rainix (ubuntu-latest, test-wasm-build)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-static)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-artifacts)
- GitHub Check: rainix (ubuntu-latest, rainix-sol-test)
- GitHub Check: rainix (macos-latest, rainix-rs-test)
- GitHub Check: rainix (ubuntu-latest, rainix-rs-test)
- GitHub Check: git-clean
|
|
||
| [target.'cfg(target_family = "wasm")'.dependencies] | ||
| getrandom = { version = "0", features = ["js", "js-sys"] } | ||
| getrandom = { version = "0.2", features = ["js", "js-sys"] } |
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.
🧩 Analysis chain
🌐 Web query:
getrandom 0.2 features js-sys crates.io documentation
💡 Result:
Short answer — getrandom 0.2 uses the feature name "js" to enable the wasm JavaScript backend (wasm32-unknown-unknown). Enabling it pulls in js-sys/wasm-bindgen and makes getrandom call the Web Crypto API (browser) or Node.js crypto methods; it should normally be enabled only for binaries/tests (not library crates). Example:
[dependencies]
getrandom = { version = "0.2", features = ["js"] }
References: docs.rs (getrandom 0.2). [1] Detailed explanation and recommendation in the crate source/readme. [2] Feature summary mentioning js-sys/wasm-bindgen. [3]
Sources:
[1] getrandom 0.2.2 — docs.rs. (docs.rs)
[2] getrandom crate lib.rs/readme (WASM/js docs). (janwalter.org)
[3] Feature flags summary (wasm_js / js-sys note). (lib.rs)
Citations:
- 1: https://docs.rs/crate/getrandom/0.2.2?utm_source=openai
- 2: https://www.janwalter.org/doc/rust/src/getrandom/lib.rs.html?utm_source=openai
- 3: https://lib.rs/crates/getrandom/features?utm_source=openai
🏁 Script executed:
# Check the actual Cargo.toml file to understand current state
cd crates/test_fixtures && cat -n Cargo.toml | grep -A 2 -B 2 "getrandom"Repository: rainlanguage/rain.interpreter
Length of output: 207
🏁 Script executed:
# Try to find getrandom in the repo's lock file or check what version is actually used
fd -t f "Cargo.lock" -o "Cargo.toml" | head -1 | xargs cat | grep -A 2 "getrandom" | head -20Repository: rainlanguage/rain.interpreter
Length of output: 212
🏁 Script executed:
cd crates/test_fixtures && cargo tree --target wasm32-unknown-unknown 2>&1 | head -30Repository: rainlanguage/rain.interpreter
Length of output: 182
🏁 Script executed:
# Try to check if Cargo validates the features
cd crates/test_fixtures && cargo check --target wasm32-unknown-unknown 2>&1 | grep -i "js-sys\|feature" | head -20Repository: rainlanguage/rain.interpreter
Length of output: 55
Remove invalid feature flag js-sys from getrandom dependency.
Getrandom 0.2 has a js feature that enables the wasm JavaScript backend and automatically pulls in js-sys as a transitive dependency. The js-sys is not itself a valid feature flag. This configuration would fail at compile time with an unknown feature error.
Apply this diff:
-getrandom = { version = "0.2", features = ["js", "js-sys"] }
+getrandom = { version = "0.2", features = ["js"] }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| getrandom = { version = "0.2", features = ["js", "js-sys"] } | |
| getrandom = { version = "0.2", features = ["js"] } |
🤖 Prompt for AI Agents
In crates/test_fixtures/Cargo.toml around line 15, the getrandom dependency
includes an invalid feature flag "js-sys"; remove "js-sys" from the features
list and leave only the valid "js" feature (e.g., features = ["js"]) so Cargo no
longer fails with an unknown feature error.
|
@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. 🧩 Analysis chain🏁 Script executed: Length of output: 559 SIZE=S |
Chained PR
svm-rsandsvm-rs-buildsdeps #423Motivation
There was an issue with updating some of the dependencies in orderbook repository that required us to use a different version of get random in this repository. This PR is created to update
getrandomdependency.Solution
Checks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.