Skip to content

Conversation

@findolor
Copy link
Contributor

@findolor findolor commented Dec 9, 2025

Chained PR

Motivation

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 getrandom dependency.

Solution

Checks

By submitting this for review, I'm confirming I've done the following:

  • made this PR as small as possible
  • unit-tested any new functionality
  • linked any relevant issues or PRs
  • included screenshots (if this involves a front-end change)

Summary by CodeRabbit

  • Chores
    • Updated a WebAssembly-related dependency version to improve compatibility and stability.

✏️ Tip: You can customize this high-level summary in your review settings.

@findolor findolor requested review from 0xgleb and hardyjosh December 9, 2025 11:57
@findolor findolor self-assigned this Dec 9, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 9, 2025

Walkthrough

The 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

Cohort / File(s) Summary
Dependency Version Update
crates/test_fixtures/Cargo.toml
Updated getrandom version from "0" to "0.2" for wasm target; features ["js", "js-sys"] unchanged

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Suggested reviewers

  • hardyjosh
  • 0xgleb

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: updating the getrandom dependency version in the test_fixtures crate from 0 to 0.2.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 2025-12-09-bump-getrandom

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@0xgleb 0xgleb left a 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

Base automatically changed from 2025-12-09-alloy-bump to main December 10, 2025 13:40
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between 16ca27f and 1a349b3.

📒 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"] }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 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:


🏁 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 -20

Repository: rainlanguage/rain.interpreter

Length of output: 212


🏁 Script executed:

cd crates/test_fixtures && cargo tree --target wasm32-unknown-unknown 2>&1 | head -30

Repository: 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 -20

Repository: 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.

Suggested change
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.

@findolor findolor merged commit 7bf57b6 into main Dec 10, 2025
11 checks passed
@github-actions
Copy link
Contributor

@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:

  • Simple bug fixes, typos, or minor refactoring
  • Single-purpose changes affecting 1-2 files
  • Documentation updates
  • Configuration tweaks
  • Changes that require minimal context to review

Review Effort: Would have taken 5-10 minutes

Examples:

  • Fix typo in variable name
  • Update README with new instructions
  • Adjust configuration values
  • Simple one-line bug fixes
  • Import statement cleanup

Medium (M)

Characteristics:

  • Feature additions or enhancements
  • Refactoring that touches multiple files but maintains existing behavior
  • Breaking changes with backward compatibility
  • Changes requiring some domain knowledge to review

Review Effort: Would have taken 15-30 minutes

Examples:

  • Add new feature or component
  • Refactor common utility functions
  • Update dependencies with minor breaking changes
  • Add new component with tests
  • Performance optimizations
  • More complex bug fixes

Large (L)

Characteristics:

  • Major feature implementations
  • Breaking changes or API redesigns
  • Complex refactoring across multiple modules
  • New architectural patterns or significant design changes
  • Changes requiring deep context and multiple review rounds

Review Effort: Would have taken 45+ minutes

Examples:

  • Complete new feature with frontend/backend changes
  • Protocol upgrades or breaking changes
  • Major architectural refactoring
  • Framework or technology upgrades

Additional Factors to Consider

When deciding between sizes, also consider:

  • Test coverage impact: More comprehensive test changes lean toward larger classification
  • Risk level: Changes to critical systems bump up a size category
  • Team familiarity: Novel patterns or technologies increase complexity

Notes:

  • the assessment must be for the totality of the PR, that means comparing the base branch to the last commit of the PR
  • the assessment output must be exactly one of: S, M or L (single-line comment) in format of: SIZE={S/M/L}
  • do not include any additional text, only the size classification
  • your assessment comment must not include tips or additional sections
  • do NOT tag me or anyone else on your comment

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 10, 2025

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Fetch the complete diff for PR #424 to assess the totality of changes
gh pr diff 424

Length of output: 559


SIZE=S

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants