Bump svm-rs and svm-rs-build deps
#2356
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
WalkthroughReplaces cloned single-element arrays with borrowed single-item slices in two Rust call sites, updates a wasm-target Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/test-webapp.yaml (1)
19-19: Remove redundant COMMIT_SHA definition in prep-webapp.sh step.COMMIT_SHA is already defined at the job level (line 19) as
${{ github.sha }}, making the explicit pass on line 63 redundant. Job-level environment variables in GitHub Actions are automatically available to all steps.Apply this diff to remove the redundant definition:
- run: ./prep-webapp.sh env: PUBLIC_WALLETCONNECT_PROJECT_ID: ${{ secrets.WALLETCONNECT_PROJECT_ID || 'test' }} - COMMIT_SHA: ${{ github.sha }} CARGO_HOME: ${{ github.workspace }}/.cargo CARGO_TARGET_DIR: ${{ github.workspace }}/.cargo/targetAlso applies to: 63-63
.github/workflows/test-ui-components.yaml (1)
19-19: Remove redundant COMMIT_SHA definition in prep-webapp.sh step.COMMIT_SHA is already defined at the job level (line 19) as
${{ github.sha }}, making the explicit pass on line 63 redundant. Job-level environment variables in GitHub Actions are automatically available to all steps.Apply this diff to remove the redundant definition:
- run: ./prep-webapp.sh env: PUBLIC_WALLETCONNECT_PROJECT_ID: ${{ secrets.WALLETCONNECT_PROJECT_ID || 'test' }} - COMMIT_SHA: ${{ github.sha }} CARGO_HOME: ${{ github.workspace }}/.cargo CARGO_TARGET_DIR: ${{ github.workspace }}/.cargo/targetAlso applies to: 63-63
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/test-ui-components.yaml(2 hunks).github/workflows/test-webapp.yaml(2 hunks)lib/rain.interpreter(1 hunks)prep-webapp.sh(1 hunks)
🧰 Additional context used
🧠 Learnings (17)
📓 Common learnings
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2000
File: crates/common/src/raindex_client/vaults.rs:183-183
Timestamp: 2025-07-16T10:40:05.717Z
Learning: In the rainlanguage/rain.orderbook codebase, user findolor considers breaking changes from Option<U256> to U256 for required fields like decimals in RaindexVaultToken to be acceptable and safe, even when they affect multiple usage sites across the codebase.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1913
File: Cargo.toml:55-55
Timestamp: 2025-06-06T16:38:04.182Z
Learning: In rain.orderbook repository, during PR chains involving dependency updates, wasm-bindgen-utils may temporarily point to git commits that don't contain the full required functionality, with the understanding that subsequent PRs in the chain will update it to the correct commit.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1956
File: crates/common/src/fuzz/mod.rs:64-64
Timestamp: 2025-07-04T09:02:57.301Z
Learning: In rainlanguage/rain.orderbook, user findolor prefers to limit type consistency changes to only the parts directly related to the current work scope. For example, when updating chain_id fields from u64 to u32 in fuzz-related code, unrelated files like tauri-app wallet commands can remain as u64 if they serve different purposes and aren't part of the current changes.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:26:31.566Z
Learning: For PR #1884 and future PRs in rainlanguage/rain.orderbook repository, provide PR size reports in a separate comment with ONLY the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number", without any additional text or explanation. Reports should exclude irrelevant files such as lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock).
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T22:14:22.646Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, I must consistently report the PR size after EVERY new commit in the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number" without any additional text or formatting blocks. The report must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock) and can be included either as part of the review or as a separate comment.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1938
File: crates/js_api/src/raindex/mod.rs:102-118
Timestamp: 2025-06-18T12:56:44.290Z
Learning: In the rainlanguage/rain.orderbook codebase, it's acceptable to scaffold unused enum variants in initial implementation PRs when they will be implemented in future PRs, as confirmed by findolor.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1744
File: crates/subgraph/src/orderbook_client.rs:44-46
Timestamp: 2025-05-14T05:13:59.713Z
Learning: In the rain.orderbook project, WASM dependencies are intentionally made available in non-WASM targets to facilitate testing of WASM-related functionality, so conditional compilation guards like `#[cfg(target_family = "wasm")]` should not be added to imports or implementations that may be needed for tests.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1710
File: crates/quote/Cargo.toml:30-30
Timestamp: 2025-05-14T04:49:14.621Z
Learning: In the rain.orderbook repository, wasm-bindgen-utils is intentionally used as an unconditional dependency because non-wasm targets (like tests) sometimes need to use it.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1715
File: crates/js_api/src/common/mod.rs:111-118
Timestamp: 2025-04-30T09:28:36.960Z
Learning: In the rain.orderbook repository, the WASM tests are already properly configured with conditional compilation using `#[cfg(target_family = "wasm")]` and `#[cfg(not(target_family = "wasm"))]`, and don't require additional `wasm_bindgen_test_configure!(run_in_browser)` directives.
Learnt from: brusherru
Repo: rainlanguage/rain.orderbook PR: 2044
File: packages/orderbook/README.md:186-189
Timestamp: 2025-08-01T09:07:20.383Z
Learning: In the rainlanguage/rain.orderbook project, Rust methods on structs like RaindexVaultsList are exported as JavaScript getters in WASM bindings using #[wasm_bindgen(getter)]. This means while the Rust code uses method calls like items(), the JavaScript/WASM API exposes them as property access like .items. The README.md correctly documents the JavaScript API surface, not the Rust implementation details.
Learnt from: CR
Repo: rainlanguage/rain.orderbook PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T16:50:31.752Z
Learning: Applies to **/*.rs : Rust: lint with `nix develop -c rainix-rs-static` (preconfigured flags included)
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2008
File: crates/common/src/raindex_client/add_orders.rs:85-86
Timestamp: 2025-07-18T10:31:05.498Z
Learning: In the rainlanguage/rain.orderbook codebase, cfg-guarded imports like `#[cfg(not(target_family = "wasm"))] use super::*;` in test modules are sometimes needed to fix compiler warnings, even when similar imports exist in nested sub-modules. These should not be removed as they serve a specific purpose in the compilation process.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 2246
File: packages/orderbook/scripts/buildEsm.js:38-53
Timestamp: 2025-10-25T21:07:19.704Z
Learning: In the rain.orderbook repository, the deduplication logic in buildEsm.js (and similar build scripts) intentionally lacks defensive checks for indexOf returning -1. If duplicates are not found, the script should throw to surface breaking changes in wasm-bindgen output that require attention.
Learnt from: CR
Repo: rainlanguage/rain.orderbook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T16:50:11.197Z
Learning: Applies to crates/**/*.rs : For Rust crates in `crates/*`, run lints using `nix develop -c cargo clippy --workspace --all-targets --all-features -D warnings`
Learnt from: brusherru
Repo: rainlanguage/rain.orderbook PR: 2036
File: crates/js_api/src/filters/raindex_filter_store.rs:446-533
Timestamp: 2025-08-02T02:34:32.237Z
Learning: In the rainlanguage/rain.orderbook project's RaindexFilterStore WASM tests (crates/js_api/src/filters/raindex_filter_store.rs), brusherru decided to focus on testing only methods without side effects (that don't use web_sys) due to difficulties with mocking localStorage and window APIs in the WASM test environment. This pragmatic approach tests pure logic separately from browser integration.
📚 Learning: 2025-07-11T12:30:09.245Z
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1971
File: .github/workflows/vercel-docs-prod.yaml:32-34
Timestamp: 2025-07-11T12:30:09.245Z
Learning: findolor prefers not to pin Node versions explicitly in GitHub Actions workflows when using Nix with webapp-shell, considering the current setup sufficient for deterministic builds in the rainlanguage/rain.orderbook project.
Applied to files:
.github/workflows/test-webapp.yaml
📚 Learning: 2025-11-25T16:50:11.197Z
Learnt from: CR
Repo: rainlanguage/rain.orderbook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T16:50:11.197Z
Learning: Before pushing, do a quick verification by running: `nix develop -c npm run lint-format-check:all`, `nix develop -c npm run test`, and `nix develop -c cargo test --workspace`
Applied to files:
.github/workflows/test-webapp.yamlprep-webapp.sh.github/workflows/test-ui-components.yaml
📚 Learning: 2025-11-25T16:50:11.197Z
Learnt from: CR
Repo: rainlanguage/rain.orderbook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T16:50:11.197Z
Learning: Applies to packages/webapp/**/*.{svelte,ts,tsx,js,jsx} : For Webapp (`packages/webapp`), run lints and format checks using `nix develop -c npm run svelte-lint-format-check -w rainlanguage/webapp`
Applied to files:
.github/workflows/test-webapp.yaml.github/workflows/test-ui-components.yaml
📚 Learning: 2025-11-25T16:50:11.197Z
Learnt from: CR
Repo: rainlanguage/rain.orderbook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T16:50:11.197Z
Learning: Before the final commit of the session, fully mirror CI by running: `./prep-all.sh`, `nix develop -c npm run lint-format-check:all`, `nix develop -c npm run build:orderbook`, `nix develop -c npm run build:ui`, `nix develop -c cargo test --workspace`, `nix develop -c npm run test`, and `nix develop -c forge test`
Applied to files:
.github/workflows/test-webapp.yamlprep-webapp.sh.github/workflows/test-ui-components.yaml
📚 Learning: 2025-11-25T16:50:31.752Z
Learnt from: CR
Repo: rainlanguage/rain.orderbook PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T16:50:31.752Z
Learning: Quick preflight check: run `nix develop -c npm run lint-format-check:all && nix develop -c rainix-rs-static` before submitting
Applied to files:
.github/workflows/test-webapp.yamlprep-webapp.sh.github/workflows/test-ui-components.yaml
📚 Learning: 2025-11-25T16:50:31.752Z
Learnt from: CR
Repo: rainlanguage/rain.orderbook PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T16:50:31.752Z
Learning: Applies to **/*.rs : Rust: lint with `nix develop -c rainix-rs-static` (preconfigured flags included)
Applied to files:
.github/workflows/test-webapp.yaml.github/workflows/test-ui-components.yaml
📚 Learning: 2025-11-25T16:50:11.197Z
Learnt from: CR
Repo: rainlanguage/rain.orderbook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T16:50:11.197Z
Learning: Applies to packages/webapp/**/*.{ts,tsx,js,jsx} : For Webapp (`packages/webapp`), run tests using `nix develop -c npm run test -w rainlanguage/webapp`
Applied to files:
.github/workflows/test-webapp.yamlprep-webapp.sh.github/workflows/test-ui-components.yaml
📚 Learning: 2025-11-25T16:50:11.197Z
Learnt from: CR
Repo: rainlanguage/rain.orderbook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T16:50:11.197Z
Learning: Applies to crates/**/*.rs : For Rust crates in `crates/*`, run lints using `nix develop -c cargo clippy --workspace --all-targets --all-features -D warnings`
Applied to files:
.github/workflows/test-webapp.yaml.github/workflows/test-ui-components.yaml
📚 Learning: 2025-11-25T16:50:11.197Z
Learnt from: CR
Repo: rainlanguage/rain.orderbook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T16:50:11.197Z
Learning: Applies to packages/ui-components/**/*.{svelte,ts,tsx,js,jsx} : For UI components (`packages/ui-components`), run lints and format checks using `nix develop -c npm run svelte-lint-format-check -w rainlanguage/ui-components`
Applied to files:
.github/workflows/test-webapp.yamlprep-webapp.sh.github/workflows/test-ui-components.yaml
📚 Learning: 2025-11-25T16:50:31.752Z
Learnt from: CR
Repo: rainlanguage/rain.orderbook PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T16:50:31.752Z
Learning: Applies to **/*.{ts,tsx,svelte} : TypeScript/Svelte: lint with `nix develop -c npm run lint`
Applied to files:
.github/workflows/test-webapp.yaml.github/workflows/test-ui-components.yaml
📚 Learning: 2025-11-25T16:50:31.752Z
Learnt from: CR
Repo: rainlanguage/rain.orderbook PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T16:50:31.752Z
Learning: Applies to **/*.rs : Rust: format code with `nix develop -c cargo fmt --all`
Applied to files:
.github/workflows/test-webapp.yaml.github/workflows/test-ui-components.yaml
📚 Learning: 2025-11-25T16:50:11.197Z
Learnt from: CR
Repo: rainlanguage/rain.orderbook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T16:50:11.197Z
Learning: Applies to packages/orderbook/**/*.{ts,tsx,js,jsx} : For Orderbook TypeScript (`packages/orderbook`), run tests using `nix develop -c npm run test -w rainlanguage/orderbook`
Applied to files:
prep-webapp.sh
📚 Learning: 2025-11-25T16:50:11.197Z
Learnt from: CR
Repo: rainlanguage/rain.orderbook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T16:50:11.197Z
Learning: Applies to packages/orderbook/**/*.{ts,tsx,js,jsx} : For Orderbook TypeScript (`packages/orderbook`), run lints using `nix develop -c npm run check -w rainlanguage/orderbook`
Applied to files:
prep-webapp.sh
📚 Learning: 2025-11-25T16:50:11.197Z
Learnt from: CR
Repo: rainlanguage/rain.orderbook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T16:50:11.197Z
Learning: If the end-of-session gate fails during `./prep-all.sh`, run the sequential fallback steps starting with `nix develop -c forge install` and following the specified prelude commands.
Applied to files:
prep-webapp.sh
📚 Learning: 2025-11-25T16:50:31.752Z
Learnt from: CR
Repo: rainlanguage/rain.orderbook PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T16:50:31.752Z
Learning: Run every command through `nix develop -c <cmd>`; avoid entering an interactive Nix shell
Applied to files:
prep-webapp.sh
📚 Learning: 2025-11-25T16:50:11.197Z
Learnt from: CR
Repo: rainlanguage/rain.orderbook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T16:50:11.197Z
Learning: Always run commands via Nix: `nix develop -c <command>`. Never cancel long-running tasks (45–90 min builds, 30+ min tests).
Applied to files:
prep-webapp.sh
🪛 Shellcheck (0.11.0)
prep-webapp.sh
[error] 35-35: Double quote array expansions to avoid re-splitting elements.
(SC2068)
⏰ 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). (17)
- GitHub Check: git-clean
- GitHub Check: test
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-legal)
- GitHub Check: standard-tests (ubuntu-latest, test-js-bindings)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-test)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-artifacts, true)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-browser-test)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-static)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-test)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-static)
- GitHub Check: standard-tests (ubuntu-latest, ob-rs-test, true)
- GitHub Check: test
- GitHub Check: build-tauri (ubuntu-22.04, true)
- GitHub Check: Deploy-Preview-Push
- GitHub Check: Deploy-Docs-Preview
🔇 Additional comments (8)
lib/rain.interpreter (1)
1-1: Wait for dependent PR #424 to merge before updating the submodule.The submodule pointer
134e1ce0afebd2bbedb31adf3e440d7333ec53c8currently points to the HEAD of PR #424 ("Updategetrandomversion intext_fixturescrate"), which is still open and unmerged. The PR objectives state to update this PR with the latest commit only after that dependent PR is merged. Update the submodule to the merge commit once PR #424 is merged.⛔ Skipped due to learnings
Learnt from: rouzwelt Repo: rainlanguage/rain.orderbook PR: 0 File: :0-0 Timestamp: 2025-05-21T22:14:22.646Z Learning: For PR #1884 in rainlanguage/rain.orderbook, I must consistently report the PR size after EVERY new commit in the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number" without any formatting blocks. The report must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock) and can be included either as part of the review or as a separate comment.Learnt from: rouzwelt Repo: rainlanguage/rain.orderbook PR: 0 File: :0-0 Timestamp: 2025-05-21T22:14:22.646Z Learning: For PR #1884 in rainlanguage/rain.orderbook, I must consistently report the PR size after EVERY new commit in the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number" without any additional text or formatting blocks. The report must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock) and can be included either as part of the review or as a separate comment.Learnt from: findolor Repo: rainlanguage/rain.orderbook PR: 1913 File: Cargo.toml:55-55 Timestamp: 2025-06-06T16:38:04.182Z Learning: In rain.orderbook repository, during PR chains involving dependency updates, wasm-bindgen-utils may temporarily point to git commits that don't contain the full required functionality, with the understanding that subsequent PRs in the chain will update it to the correct commit.prep-webapp.sh (1)
32-32: LGTM: Strategic cleanup steps before builds.The placement of
rm -rf .cargo target || truebefore npm build blocks is a good strategy to free disk space before Rust/WASM compilation, particularly relevant for the svm-rs dependency update. The|| trueproperly handles cases where these directories may not exist.Also applies to: 37-37
.github/workflows/test-webapp.yaml (3)
26-41: LGTM: Comprehensive disk cleanup strategy.The "Free disk space" step aggressively removes system toolchains and directories that bloat the GitHub Actions runner. This is well-suited to address disk constraints when building Rust artifacts.
58-58: LGTM: gc-max-store-size-linux increase aligns with cleanup strategy.Increasing the Nix garbage-collection threshold from 1G to 10G complements the disk cleanup efforts and provides more headroom for Rust/WASM build caching, supporting the svm-rs dependency updates.
64-65: LGTM: Workspace-isolated Cargo setup and cleanup alignment.Setting CARGO_HOME and CARGO_TARGET_DIR to workspace-relative paths (
.cargoand.cargo/target) ensures CI builds are isolated and allows the cleanup step (line 68) to efficiently remove all build artifacts in a single operation.Also applies to: 67-68
.github/workflows/test-ui-components.yaml (3)
26-41: LGTM: Consistent disk cleanup across workflows.The "Free disk space" step mirrors the one in test-webapp.yaml, ensuring consistent resource cleanup strategies across UI-related CI workflows.
58-58: LGTM: gc-max-store-size-linux increase consistent with test-webapp.yaml.The increase to 10G provides consistent caching headroom across workflows and supports the Rust dependency updates.
64-65: LGTM: Workspace-isolated Cargo setup and cleanup alignment.Setting CARGO_HOME and CARGO_TARGET_DIR to workspace-relative paths ensures CI builds are isolated and the cleanup step efficiently removes all build artifacts.
Also applies to: 67-68
| rm -rf .cargo target || true | ||
|
|
||
| echo "Building packages..." | ||
| nix develop -i ${keep[@]} -c bash -c '(npm run build -w @rainlanguage/orderbook)' |
Contributor
There was a problem hiding this comment.
Fix Shellcheck SC2068 warning by quoting array expansion.
The array ${keep[@]} should be quoted as "${keep[@]}" to prevent word-splitting if environment variables contain spaces. This could cause the nix develop invocation to fail unexpectedly.
Apply this diff to fix the array expansion:
-nix develop -i ${keep[@]} -c bash -c '(npm run build -w @rainlanguage/orderbook)'
+nix develop -i "${keep[@]}" -c bash -c '(npm run build -w @rainlanguage/orderbook)'Additionally, apply the same fix to line 39 if it also uses ${keep[@]}:
-nix develop -i ${keep[@]} -c bash -c '(npm run build -w @rainlanguage/ui-components && npm run build -w @rainlanguage/webapp)'
+nix develop -i "${keep[@]}" -c bash -c '(npm run build -w @rainlanguage/ui-components && npm run build -w @rainlanguage/webapp)'📝 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
| nix develop -i ${keep[@]} -c bash -c '(npm run build -w @rainlanguage/orderbook)' | |
| nix develop -i "${keep[@]}" -c bash -c '(npm run build -w @rainlanguage/orderbook)' |
🧰 Tools
🪛 Shellcheck (0.11.0)
[error] 35-35: Double quote array expansions to avoid re-splitting elements.
(SC2068)
🤖 Prompt for AI Agents
In prep-webapp.sh around line 35 (and also update line 39 if it uses the same
expansion), the array expansion ${keep[@]} is unquoted which triggers ShellCheck
SC2068 and can cause word-splitting when elements contain spaces; change the
invocation to quote the array expansion as "${keep[@]}" so each array element is
preserved as a single argument to nix develop (apply the same quoted expansion
on line 39 if present).
0xgleb
approved these changes
Dec 9, 2025
Comment on lines
26
to
41
| - name: Free disk space | ||
| run: | | ||
| sudo rm -rf \ | ||
| "$AGENT_TOOLSDIRECTORY" \ | ||
| /opt/microsoft/msedge \ | ||
| /opt/microsoft/powershell \ | ||
| /opt/pipx \ | ||
| /usr/lib/mono \ | ||
| /usr/local/julia* \ | ||
| /usr/local/lib/android \ | ||
| /usr/local/lib/node_modules \ | ||
| /usr/local/share/powershell \ | ||
| /usr/share/dotnet \ | ||
| /usr/share/swift \ | ||
| /opt/google/chrome \ | ||
| /usr/local/share/chromium |
Collaborator
There was a problem hiding this comment.
Consider using a special GH Action
Suggested change
| - name: Free disk space | |
| run: | | |
| sudo rm -rf \ | |
| "$AGENT_TOOLSDIRECTORY" \ | |
| /opt/microsoft/msedge \ | |
| /opt/microsoft/powershell \ | |
| /opt/pipx \ | |
| /usr/lib/mono \ | |
| /usr/local/julia* \ | |
| /usr/local/lib/android \ | |
| /usr/local/lib/node_modules \ | |
| /usr/local/share/powershell \ | |
| /usr/share/dotnet \ | |
| /usr/share/swift \ | |
| /opt/google/chrome \ | |
| /usr/local/share/chromium | |
| - name: Free disk space | |
| uses: jlumbroso/free-disk-space@v1.3.1 | |
| with: | |
| android: true | |
| dotnet: true | |
| haskell: true | |
| large-packages: false | |
| docker-images: false | |
| swap-storage: false |
| submodules: recursive | ||
| fetch-depth: 0 | ||
|
|
||
| - name: Free disk space |
hardyjosh
approved these changes
Dec 10, 2025
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
.github/workflows/vercel-preview.yaml (1)
40-41: Consider specifying jlumbroso/free-disk-space action parameters for maximum disk recovery.The action is used without explicit configuration, relying on defaults. Based on prior review feedback, consider adding parameters to maximize disk space recovery for this build environment.
Apply this diff to add action parameters:
- name: Free disk space - uses: jlumbroso/free-disk-space@v1.3.1 + uses: jlumbroso/free-disk-space@v1.3.1 + with: + android: true + dotnet: true + haskell: true + large-packages: false + docker-images: false + swap-storage: falseThis aligns with the pattern suggested in previous reviews and ensures maximum disk space availability for the Nix build cache and compilation artifacts.
.github/workflows/vercel-prod.yaml (1)
23-24: Consider specifying jlumbroso/free-disk-space action parameters for maximum disk recovery.The action is used without explicit configuration, relying on defaults. Based on prior review feedback, consider adding parameters to maximize disk space recovery for this build environment.
Apply this diff to add action parameters:
- name: Free disk space - uses: jlumbroso/free-disk-space@v1.3.1 + uses: jlumbroso/free-disk-space@v1.3.1 + with: + android: true + dotnet: true + haskell: true + large-packages: false + docker-images: false + swap-storage: falseThis aligns with the pattern suggested in previous reviews and ensures maximum disk space availability for the Nix build cache and compilation artifacts.
.github/workflows/test-ui-components.yaml (1)
26-27: Consider specifying jlumbroso/free-disk-space action parameters for maximum disk recovery.The action is used without explicit configuration, relying on defaults. Based on prior review feedback, consider adding parameters to maximize disk space recovery for this build environment.
Apply this diff to add action parameters:
- name: Free disk space - uses: jlumbroso/free-disk-space@v1.3.1 + uses: jlumbroso/free-disk-space@v1.3.1 + with: + android: true + dotnet: true + haskell: true + large-packages: false + docker-images: false + swap-storage: falseThis aligns with the pattern suggested in previous reviews and ensures maximum disk space availability for the Nix build cache and compilation artifacts.
.github/workflows/test-webapp.yaml (1)
26-27: Consider specifying jlumbroso/free-disk-space action parameters for maximum disk recovery.The action is used without explicit configuration, relying on defaults. Based on prior review feedback, consider adding parameters to maximize disk space recovery for this build environment.
Apply this diff to add action parameters:
- name: Free disk space - uses: jlumbroso/free-disk-space@v1.3.1 + uses: jlumbroso/free-disk-space@v1.3.1 + with: + android: true + dotnet: true + haskell: true + large-packages: false + docker-images: false + swap-storage: falseThis aligns with the pattern suggested in previous reviews and ensures maximum disk space availability for the Nix build cache and compilation artifacts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
flake.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
.github/workflows/test-ui-components.yaml(2 hunks).github/workflows/test-webapp.yaml(2 hunks).github/workflows/vercel-docs-preview.yaml(1 hunks).github/workflows/vercel-preview.yaml(1 hunks).github/workflows/vercel-prod.yaml(1 hunks)
🧰 Additional context used
🧠 Learnings (14)
📓 Common learnings
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1913
File: Cargo.toml:55-55
Timestamp: 2025-06-06T16:38:04.182Z
Learning: In rain.orderbook repository, during PR chains involving dependency updates, wasm-bindgen-utils may temporarily point to git commits that don't contain the full required functionality, with the understanding that subsequent PRs in the chain will update it to the correct commit.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 2000
File: crates/common/src/raindex_client/vaults.rs:183-183
Timestamp: 2025-07-16T10:40:05.717Z
Learning: In the rainlanguage/rain.orderbook codebase, user findolor considers breaking changes from Option<U256> to U256 for required fields like decimals in RaindexVaultToken to be acceptable and safe, even when they affect multiple usage sites across the codebase.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:26:31.566Z
Learning: For PR #1884 and future PRs in rainlanguage/rain.orderbook repository, provide PR size reports in a separate comment with ONLY the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number", without any additional text or explanation. Reports should exclude irrelevant files such as lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock).
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:53.708Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, provide a separate comment after each review with PR size statistics in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding lock files like package-lock.json and cargo.lock.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1956
File: crates/common/src/fuzz/mod.rs:64-64
Timestamp: 2025-07-04T09:02:57.301Z
Learning: In rainlanguage/rain.orderbook, user findolor prefers to limit type consistency changes to only the parts directly related to the current work scope. For example, when updating chain_id fields from u64 to u32 in fuzz-related code, unrelated files like tauri-app wallet commands can remain as u64 if they serve different purposes and aren't part of the current changes.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T22:14:22.646Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, I must consistently report the PR size after EVERY new commit in the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number" without any additional text or formatting blocks. The report must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock) and can be included either as part of the review or as a separate comment.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T23:09:46.344Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, I must reassess the PR size after each new commit to ensure the report includes the latest changes, and calculate the total size up to the very latest commit.
Learnt from: findolor
Repo: rainlanguage/rain.orderbook PR: 1744
File: crates/subgraph/src/orderbook_client.rs:44-46
Timestamp: 2025-05-14T05:13:59.713Z
Learning: In the rain.orderbook project, WASM dependencies are intentionally made available in non-WASM targets to facilitate testing of WASM-related functionality, so conditional compilation guards like `#[cfg(target_family = "wasm")]` should not be added to imports or implementations that may be needed for tests.
Learnt from: brusherru
Repo: rainlanguage/rain.orderbook PR: 2036
File: crates/js_api/src/filters/raindex_filter_store.rs:446-533
Timestamp: 2025-08-02T02:34:32.237Z
Learning: In the rainlanguage/rain.orderbook project's RaindexFilterStore WASM tests (crates/js_api/src/filters/raindex_filter_store.rs), brusherru decided to focus on testing only methods without side effects (that don't use web_sys) due to difficulties with mocking localStorage and window APIs in the WASM test environment. This pragmatic approach tests pure logic separately from browser integration.
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 2246
File: packages/orderbook/scripts/buildEsm.js:38-53
Timestamp: 2025-10-25T21:07:19.704Z
Learning: In the rain.orderbook repository, the deduplication logic in buildEsm.js (and similar build scripts) intentionally lacks defensive checks for indexOf returning -1. If duplicates are not found, the script should throw to surface breaking changes in wasm-bindgen output that require attention.
Learnt from: CR
Repo: rainlanguage/rain.orderbook PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T16:50:31.752Z
Learning: Applies to **/*.rs : Rust: lint with `nix develop -c rainix-rs-static` (preconfigured flags included)
📚 Learning: 2025-11-25T16:50:31.752Z
Learnt from: CR
Repo: rainlanguage/rain.orderbook PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T16:50:31.752Z
Learning: Quick preflight check: run `nix develop -c npm run lint-format-check:all && nix develop -c rainix-rs-static` before submitting
Applied to files:
.github/workflows/test-webapp.yaml.github/workflows/test-ui-components.yaml
📚 Learning: 2025-11-25T16:50:11.197Z
Learnt from: CR
Repo: rainlanguage/rain.orderbook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T16:50:11.197Z
Learning: Before pushing, do a quick verification by running: `nix develop -c npm run lint-format-check:all`, `nix develop -c npm run test`, and `nix develop -c cargo test --workspace`
Applied to files:
.github/workflows/test-webapp.yaml.github/workflows/test-ui-components.yaml
📚 Learning: 2025-11-25T16:50:11.197Z
Learnt from: CR
Repo: rainlanguage/rain.orderbook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T16:50:11.197Z
Learning: Applies to packages/webapp/**/*.{svelte,ts,tsx,js,jsx} : For Webapp (`packages/webapp`), run lints and format checks using `nix develop -c npm run svelte-lint-format-check -w rainlanguage/webapp`
Applied to files:
.github/workflows/test-webapp.yaml.github/workflows/test-ui-components.yaml
📚 Learning: 2025-11-25T16:50:11.197Z
Learnt from: CR
Repo: rainlanguage/rain.orderbook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T16:50:11.197Z
Learning: Before the final commit of the session, fully mirror CI by running: `./prep-all.sh`, `nix develop -c npm run lint-format-check:all`, `nix develop -c npm run build:orderbook`, `nix develop -c npm run build:ui`, `nix develop -c cargo test --workspace`, `nix develop -c npm run test`, and `nix develop -c forge test`
Applied to files:
.github/workflows/test-webapp.yaml.github/workflows/test-ui-components.yaml
📚 Learning: 2025-11-25T16:50:31.752Z
Learnt from: CR
Repo: rainlanguage/rain.orderbook PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T16:50:31.752Z
Learning: Applies to **/*.rs : Rust: lint with `nix develop -c rainix-rs-static` (preconfigured flags included)
Applied to files:
.github/workflows/test-webapp.yaml.github/workflows/test-ui-components.yaml
📚 Learning: 2025-11-25T16:50:11.197Z
Learnt from: CR
Repo: rainlanguage/rain.orderbook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T16:50:11.197Z
Learning: Applies to crates/**/*.rs : For Rust crates in `crates/*`, run lints using `nix develop -c cargo clippy --workspace --all-targets --all-features -D warnings`
Applied to files:
.github/workflows/test-webapp.yaml.github/workflows/test-ui-components.yaml
📚 Learning: 2025-11-25T16:50:11.197Z
Learnt from: CR
Repo: rainlanguage/rain.orderbook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T16:50:11.197Z
Learning: Applies to packages/webapp/**/*.{ts,tsx,js,jsx} : For Webapp (`packages/webapp`), run tests using `nix develop -c npm run test -w rainlanguage/webapp`
Applied to files:
.github/workflows/test-webapp.yaml.github/workflows/test-ui-components.yaml
📚 Learning: 2025-11-25T16:50:11.197Z
Learnt from: CR
Repo: rainlanguage/rain.orderbook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T16:50:11.197Z
Learning: Applies to packages/ui-components/**/*.{svelte,ts,tsx,js,jsx} : For UI components (`packages/ui-components`), run lints and format checks using `nix develop -c npm run svelte-lint-format-check -w rainlanguage/ui-components`
Applied to files:
.github/workflows/test-webapp.yaml.github/workflows/test-ui-components.yaml
📚 Learning: 2025-11-25T16:50:31.752Z
Learnt from: CR
Repo: rainlanguage/rain.orderbook PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T16:50:31.752Z
Learning: Applies to **/*.rs : Rust: format code with `nix develop -c cargo fmt --all`
Applied to files:
.github/workflows/test-webapp.yaml.github/workflows/test-ui-components.yaml
📚 Learning: 2025-11-25T16:50:11.197Z
Learnt from: CR
Repo: rainlanguage/rain.orderbook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T16:50:11.197Z
Learning: Applies to crates/**/*.rs : For Rust crates in `crates/*`, run tests using `nix develop -c cargo test --workspace` or `--package <crate>`
Applied to files:
.github/workflows/test-webapp.yaml
📚 Learning: 2025-05-21T21:24:42.275Z
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:42.275Z
Learning: For PR #1884, after each commit review, I should provide a new comment with a PR size report in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`, excluding irrelevant files such as lock files (package-lock.json, cargo.lock, etc.).
Applied to files:
.github/workflows/test-ui-components.yaml
📚 Learning: 2025-05-21T21:24:42.275Z
Learnt from: rouzwelt
Repo: rainlanguage/rain.orderbook PR: 0
File: :0-0
Timestamp: 2025-05-21T21:24:42.275Z
Learning: After each commit in PR #1884, I should provide a new comment with the PR size report in the format: `TOTAL=number ADDITIONS=number DELETIONS=number`. The report should exclude irrelevant files such as lock files (e.g., package-lock.json, cargo.lock).
Applied to files:
.github/workflows/test-ui-components.yaml
📚 Learning: 2025-11-25T16:50:31.752Z
Learnt from: CR
Repo: rainlanguage/rain.orderbook PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T16:50:31.752Z
Learning: Applies to **/*.{ts,tsx,svelte} : TypeScript/Svelte: lint with `nix develop -c npm run lint`
Applied to files:
.github/workflows/test-ui-components.yaml
⏰ 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). (18)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-browser-test)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-legal)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-test)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, ob-rs-test, true)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-test)
- GitHub Check: standard-tests (ubuntu-latest, test-js-bindings)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-artifacts, true)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-static)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-static)
- GitHub Check: git-clean
- GitHub Check: test
- GitHub Check: test
- GitHub Check: test
- GitHub Check: Deploy-Preview-Push
- GitHub Check: build-tauri (ubuntu-22.04, true)
- GitHub Check: Deploy-Docs-Preview
🔇 Additional comments (4)
.github/workflows/test-ui-components.yaml (2)
44-44: Verify the gc-max-store-size-linux increase from 1G to 10G.The 10x increase in the garbage collection threshold may cause the Nix cache to consume more CI runner disk space. Confirm this change is intentional and will not cause out-of-disk errors during builds.
Consider verifying that the combined effect of the free-disk-space action and increased gc-max-store-size-linux threshold maintains adequate free disk space throughout the build.
49-51: Remove the redundant cleanup step; artifacts are already cleaned by prep-webapp.sh.The environment variables
CARGO_TARGET_DIRandCARGO_HOMEset on lines 49-50 are not passed through thenix develop -iisolation in prep-webapp.sh (they're not in the-kkeep list). Cargo writes to the default./targetdirectory, which prep-webapp.sh already removes twice withrm -rf target || true. The cleanup on line 54 is redundant and can be removed..github/workflows/test-webapp.yaml (2)
44-44: Verify the gc-max-store-size-linux increase from 1G to 10G.The 10x increase in the garbage collection threshold may cause the Nix cache to consume more CI runner disk space. Confirm this change is intentional and will not cause out-of-disk errors during builds.
Consider verifying that the combined effect of the free-disk-space action and increased gc-max-store-size-linux threshold maintains adequate free disk space throughout the build.
49-51: The cleanup step correctly targets thetargetdirectory where Cargo writes artifacts by default. AlthoughCARGO_TARGET_DIRis set to.cargo/target, it is not used by the build process and can be removed.
| - run: ./prep-webapp.sh | ||
| env: | ||
| PUBLIC_WALLETCONNECT_PROJECT_ID: ${{ secrets.WALLETCONNECT_PROJECT_ID || 'test' }} | ||
| COMMIT_SHA: ${{ github.sha }} |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove redundant COMMIT_SHA environment variable.
Line 49 passes COMMIT_SHA: ${{ github.sha }} to the prep-webapp.sh step, but this variable is already defined in the job's env block at line 19. Passing it again is redundant.
Apply this diff to remove the duplicate:
- run: ./prep-webapp.sh
env:
PUBLIC_WALLETCONNECT_PROJECT_ID: ${{ secrets.WALLETCONNECT_PROJECT_ID || 'test' }}
- COMMIT_SHA: ${{ github.sha }}
CARGO_HOME: ${{ github.workspace }}/.cargo
CARGO_TARGET_DIR: ${{ github.workspace }}/.cargo/targetAlternatively, if prep-webapp.sh requires it to be passed explicitly, add a comment explaining why it's not inherited from the job env.
📝 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
| COMMIT_SHA: ${{ github.sha }} | |
| - run: ./prep-webapp.sh | |
| env: | |
| PUBLIC_WALLETCONNECT_PROJECT_ID: ${{ secrets.WALLETCONNECT_PROJECT_ID || 'test' }} | |
| CARGO_HOME: ${{ github.workspace }}/.cargo | |
| CARGO_TARGET_DIR: ${{ github.workspace }}/.cargo/target |
🤖 Prompt for AI Agents
.github/workflows/test-ui-components.yaml around line 49: the step is
redundantly passing COMMIT_SHA: ${{ github.sha }} to the prep-webapp.sh step
even though COMMIT_SHA is already defined in the job-level env at line 19;
remove the duplicate COMMIT_SHA entry from the step to avoid redundancy, or if
prep-webapp.sh truly requires an explicit step-level env, replace the duplicate
with a short comment explaining why the value must be passed again (e.g.,
"explicitly passed because step runs in a separate environment and does not
inherit job env") so the intent is clear.
| - run: ./prep-webapp.sh | ||
| env: | ||
| PUBLIC_WALLETCONNECT_PROJECT_ID: ${{ secrets.WALLETCONNECT_PROJECT_ID || 'test' }} | ||
| COMMIT_SHA: ${{ github.sha }} |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove redundant COMMIT_SHA environment variable.
Line 49 passes COMMIT_SHA: ${{ github.sha }} to the prep-webapp.sh step, but this variable is already defined in the job's env block at line 19. Passing it again is redundant.
Apply this diff to remove the duplicate:
- run: ./prep-webapp.sh
env:
PUBLIC_WALLETCONNECT_PROJECT_ID: ${{ secrets.WALLETCONNECT_PROJECT_ID || 'test' }}
- COMMIT_SHA: ${{ github.sha }}
CARGO_HOME: ${{ github.workspace }}/.cargo
CARGO_TARGET_DIR: ${{ github.workspace }}/.cargo/targetAlternatively, if prep-webapp.sh requires it to be passed explicitly, add a comment explaining why it's not inherited from the job env.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
.github/workflows/test-webapp.yaml around line 49: the step passes COMMIT_SHA:
${{ github.sha }} even though COMMIT_SHA is already set in the job's env at line
19, so remove the duplicate environment mapping from the step to avoid
redundancy; if the prep-webapp.sh step truly needs an explicit env declaration
because it does not inherit job envs in your setup, instead keep the line but
add a one-line comment explaining why it must be passed explicitly (e.g.,
"explicitly passed because step runs in a different context and does not inherit
job envs").
Comment on lines
+27
to
+28
| - name: Free disk space | ||
| uses: jlumbroso/free-disk-space@v1.3.1 |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider specifying jlumbroso/free-disk-space action parameters for maximum disk recovery.
The action is used without explicit configuration, relying on defaults. Based on prior review feedback, consider adding parameters to maximize disk space recovery for this build environment.
Apply this diff to add action parameters:
- name: Free disk space
- uses: jlumbroso/free-disk-space@v1.3.1
+ uses: jlumbroso/free-disk-space@v1.3.1
+ with:
+ android: true
+ dotnet: true
+ haskell: true
+ large-packages: false
+ docker-images: false
+ swap-storage: falseThis aligns with the pattern suggested in previous reviews and ensures maximum disk space availability for the Nix build cache and compilation artifacts.
🤖 Prompt for AI Agents
In .github/workflows/vercel-docs-preview.yaml around lines 27-28, the
jlumbroso/free-disk-space action is invoked with defaults; modify the step to
include a with: block that explicitly targets common large locations and
requests a high free-space threshold so the runner reclaims maximum disk (for
example, set paths to include /home/runner/.cache, /tmp, and
/opt/hostedtoolcache and set a min-free-mb value such as 20000 or another
project-appropriate threshold), optionally enabling any aggressive/recursive
cleanup flags the action supports; ensure the new with: parameters are added
under the action step so the build has maximum disk recovery for Nix cache and
compilation artifacts.
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:
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:
|
Contributor
|
Tip For best results, initiate chat on the files or code changes. SIZE=M 🧠 Learnings used |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Dependent PRs
getrandomversion intext_fixturescrate rain.interpreter#424Motivation
Recently there was a bug with alloy (alloy-rs/svm-rs#179) and because of this our CI and local dev environment were having issues with compilation. We needed to bump the
svm-rsandsvm-rs-buildsdeps to the new release that fixes the issue. This also required us to bump Rust version.Solution
rainix-rs-staticoutputChecks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit
Chores
Performance
Dependencies
✏️ Tip: You can customize this high-level summary in your review settings.