Conversation
|
Warning Rate limit exceeded@ananas-block has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 1 minutes and 32 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
WalkthroughAdds release CI workflows and helper scripts; removes a legacy release workflow/script; updates workspace manifests and many crate versions; adds two test workspace crates and many test files (removing feature gates); adjusts CI test matrices and marks numerous packages Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant GH as GitHub
participant WFv as "Release PR Validation" WF
participant WFp as "Publish Release" WF
participant Repo as Repo
participant Rust as Rust toolchain
participant Scripts as scripts/*.sh
participant CargoRel as cargo-release
participant GHCLI as gh
Dev->>GH: open/update PR to main with "release" label
GH-->>WFv: trigger release-pr-validation on labeled PR
WFv->>Repo: checkout (fetch-depth:0)
WFv->>Rust: setup stable toolchain
WFv->>Scripts: run validate-packages.sh (dry-run)
Scripts->>Repo: detect-version-changes.sh (base..head)
Scripts->>CargoRel: cargo release publish (dry-run)
Scripts-->>WFv: validation result
WFv-->>GH: report status
Note over GH,WFp: On PR merge with "release" label
GH-->>WFp: trigger publish workflow
WFp->>Repo: checkout
WFp->>Rust: setup stable toolchain
WFp->>Scripts: validate-packages.sh (dry-run)
WFp->>Scripts: validate-packages.sh --execute (with token)
WFp->>Scripts: detect-version-changes.sh -> list changed packages
WFp->>GHCLI: gh release create per-package tag/notes
GHCLI-->>WFp: report per-package outcome
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
1d8cf47 to
ed37ded
Compare
|
Caution No docstrings were generated. |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/release-pr-validation.yml(1 hunks).github/workflows/release-publish.yml(1 hunks)Cargo.toml(1 hunks)scripts/release-program-libs.sh(1 hunks)scripts/release-sdk-libs.sh(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). (10)
- GitHub Check: Test program-libs-fast
- GitHub Check: Test program-libs-slow
- GitHub Check: Test concurrent-merkle-tree
- GitHub Check: Test batched-merkle-tree-simulate
- GitHub Check: Forester e2e test
- GitHub Check: lint
- GitHub Check: stateless-js-v2
- GitHub Check: stateless-js-v1
- GitHub Check: cli-v2
- GitHub Check: cli-v1
| permissions: | ||
| contents: write | ||
|
|
There was a problem hiding this comment.
Grant issues: write so the PR comment succeeds.
The final github-script step posts a PR comment. With only contents: write, GitHub returns 403 and the workflow fails to report success. Please add the required scope.
runs-on: ubuntu-latest
permissions:
- contents: write
+ contents: write
+ issues: write🤖 Prompt for AI Agents
.github/workflows/release-publish.yml around lines 14-16: the workflow only
grants contents: write which causes the github-script step that posts a PR
comment to fail with 403; update the workflow permissions to include issues:
write (in addition to contents: write) so the action can create/update PR
comments — add the issues: write permission under permissions.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
.github/workflows/release-publish.yml (1)
14-16: Grantissues: writefor PR comments.The final
github-scriptstep posts to the PR, but the workflow only requestscontents: write, so creating the comment returns 403 and the publish job fails. Addissues: writeto the permissions block.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (50)
.github/workflows/release-pr-validation.yml(1 hunks).github/workflows/release-publish.yml(1 hunks).github/workflows/rust.yml(2 hunks).github/workflows/sdk-tests.yml(1 hunks)Cargo.toml(5 hunks)anchor-programs/system/Cargo.toml(1 hunks)forester-utils/Cargo.toml(1 hunks)forester/Cargo.toml(1 hunks)program-libs/batched-merkle-tree/Cargo.toml(1 hunks)program-libs/compressed-account/Cargo.toml(1 hunks)program-libs/merkle-tree-metadata/Cargo.toml(1 hunks)program-libs/verifier/Cargo.toml(1 hunks)program-libs/zero-copy-derive/Cargo.toml(2 hunks)program-libs/zero-copy/Cargo.toml(2 hunks)program-libs/zero-copy/tests/derive/instruction_data.rs(1 hunks)program-libs/zero-copy/tests/derive/main.rs(1 hunks)program-libs/zero-copy/tests/derive/random.rs(1 hunks)program-libs/zero-copy/tests/derive/ui.rs(1 hunks)program-libs/zero-copy/tests/derive/ui/fail_mut/missing_repr_c_mut.stderr(1 hunks)program-tests/account-compression-test/Cargo.toml(1 hunks)program-tests/batched-merkle-tree-test/Cargo.toml(1 hunks)program-tests/batched-merkle-tree-test/src/lib.rs(1 hunks)program-tests/batched-merkle-tree-test/tests/account_access.rs(0 hunks)program-tests/batched-merkle-tree-test/tests/merkle_tree.rs(0 hunks)program-tests/batched-merkle-tree-test/tests/queue.rs(0 hunks)program-tests/batched-merkle-tree-test/tests/rollover_address_tree.rs(0 hunks)program-tests/batched-merkle-tree-test/tests/rollover_state_tree.rs(0 hunks)program-tests/compressed-token-test/Cargo.toml(1 hunks)program-tests/create-address-test-program/Cargo.toml(1 hunks)program-tests/e2e-test/Cargo.toml(1 hunks)program-tests/registry-test/Cargo.toml(1 hunks)program-tests/system-cpi-test/Cargo.toml(1 hunks)program-tests/system-cpi-v2-test/Cargo.toml(1 hunks)program-tests/system-test/Cargo.toml(1 hunks)program-tests/utils/Cargo.toml(1 hunks)programs/account-compression/Cargo.toml(1 hunks)programs/compressed-token/Cargo.toml(1 hunks)programs/registry/Cargo.toml(1 hunks)programs/system/Cargo.toml(1 hunks)scripts/create-release-pr.sh(1 hunks)sdk-libs/client/Cargo.toml(1 hunks)sdk-libs/program-test/Cargo.toml(1 hunks)sdk-tests/client-test/Cargo.toml(1 hunks)sdk-tests/sdk-anchor-test/programs/sdk-anchor-test/Cargo.toml(1 hunks)sdk-tests/sdk-native-test/Cargo.toml(1 hunks)sdk-tests/sdk-pinocchio-v1-test/Cargo.toml(1 hunks)sdk-tests/sdk-pinocchio-v2-test/Cargo.toml(1 hunks)sdk-tests/sdk-v1-native-test/Cargo.toml(1 hunks)sparse-merkle-tree/Cargo.toml(1 hunks)xtask/Cargo.toml(1 hunks)
💤 Files with no reviewable changes (5)
- program-tests/batched-merkle-tree-test/tests/account_access.rs
- program-tests/batched-merkle-tree-test/tests/queue.rs
- program-tests/batched-merkle-tree-test/tests/merkle_tree.rs
- program-tests/batched-merkle-tree-test/tests/rollover_state_tree.rs
- program-tests/batched-merkle-tree-test/tests/rollover_address_tree.rs
✅ Files skipped from review due to trivial changes (25)
- program-libs/zero-copy/tests/derive/main.rs
- program-tests/system-test/Cargo.toml
- xtask/Cargo.toml
- sparse-merkle-tree/Cargo.toml
- sdk-tests/sdk-anchor-test/programs/sdk-anchor-test/Cargo.toml
- sdk-libs/program-test/Cargo.toml
- sdk-tests/sdk-pinocchio-v1-test/Cargo.toml
- programs/system/Cargo.toml
- program-tests/create-address-test-program/Cargo.toml
- program-tests/utils/Cargo.toml
- program-tests/system-cpi-v2-test/Cargo.toml
- program-tests/account-compression-test/Cargo.toml
- program-tests/registry-test/Cargo.toml
- program-libs/merkle-tree-metadata/Cargo.toml
- sdk-tests/client-test/Cargo.toml
- sdk-tests/sdk-native-test/Cargo.toml
- programs/account-compression/Cargo.toml
- program-libs/zero-copy/tests/derive/ui/fail_mut/missing_repr_c_mut.stderr
- sdk-libs/client/Cargo.toml
- programs/registry/Cargo.toml
- forester/Cargo.toml
- sdk-tests/sdk-v1-native-test/Cargo.toml
- programs/compressed-token/Cargo.toml
- program-tests/compressed-token-test/Cargo.toml
- program-tests/e2e-test/Cargo.toml
🧰 Additional context used
🧬 Code graph analysis (1)
program-libs/zero-copy/tests/derive/random.rs (1)
program-tests/batched-merkle-tree-test/tests/merkle_tree.rs (1)
instruction_data(378-383)
🪛 GitHub Actions: Lint GitHub Actions workflows
.github/workflows/release-publish.yml
[warning] 57-57: shellcheck reported issue: SC2188: This redirection doesn't have a command. Move to its command (or use 'true' as no-op)
[warning] 102-102: shellcheck reported issue: SC2188: This redirection doesn't have a command. Move to its command (or use 'true' as no-op)
[error] 1-1: Process completed with exit code 1.
⏰ 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). (22)
- GitHub Check: cli-v2
- GitHub Check: system-programs (sdk-anchor-test-program, ["cargo-test-sbf -p sdk-anchor-test", "cargo-test-sbf -...
- GitHub Check: system-programs (sdk-libs, light-sdk-macros light-sdk light-program-test light-client batched-mer...
- GitHub Check: system-programs (sdk-test-program, ["cargo-test-sbf -p sdk-native-test", "cargo-test-sbf -p sdk-v...
- GitHub Check: lint
- GitHub Check: stateless-js-v1
- GitHub Check: Forester e2e test
- GitHub Check: system-programs (system-cpi-test-v2-event, ["cargo-test-sbf -p system-cpi-v2-test -- event::parse"])
- GitHub Check: system-programs (light-compressed-token, ["cargo-test-sbf -p compressed-token-test"])
- GitHub Check: Test concurrent-merkle-tree
- GitHub Check: Test batched-merkle-tree-simulate
- GitHub Check: Test program-libs-fast
- GitHub Check: Test program-libs-slow
- GitHub Check: cli-v1
- GitHub Check: stateless-js-v2
- GitHub Check: system-programs (light-system-program, ["cargo-test-sbf -p system-test"])
- GitHub Check: system-programs (random-e2e-test, ["cargo-test-sbf -p e2e-test"])
- GitHub Check: system-programs (light-registry, ["cargo-test-sbf -p registry-test"])
- GitHub Check: system-programs (system-cpi-test-v2-functional, ["cargo-test-sbf -p system-cpi-v2-test -- functio...
- GitHub Check: system-programs (system-cpi-test-v2-other, ["cargo-test-sbf -p system-cpi-v2-test -- --skip funct...
- GitHub Check: system-programs (account-compression, ["cargo-test-sbf -p account-compression-test"])
- GitHub Check: system-programs (system-cpi-test, ["cargo-test-sbf -p system-cpi-test", "cargo test -p light-syst...
🔇 Additional comments (3)
program-libs/zero-copy/tests/derive/instruction_data.rs (1)
18-18: LGTM!Adding
Defaultto the derive list is appropriate for thePubkeystruct. Since it wraps a[u8; 32]array which implementsDefault, the derived implementation will initialize all bytes to zero.program-libs/zero-copy/tests/derive/ui.rs (1)
9-21: LGTM!The test path updates consistently reflect the restructuring from
tests/ui/...totests/derive/ui/.... The changes align with the PR's module reorganization objectives.program-libs/zero-copy/tests/derive/random.rs (1)
12-12: LGTM!The module import change correctly refactors
instruction_datafrom a local submodule declaration to a parent module import. This aligns with the PR's test structure reorganization whereinstruction_datais now declared at the parent level.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/release-pr-validation.yml(1 hunks).github/workflows/release-publish.yml(1 hunks)scripts/create-release-pr.sh(1 hunks)scripts/detect-version-changes.sh(1 hunks)scripts/validate-packages.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.11.0)
scripts/create-release-pr.sh
[warning] 40-40: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 41-41: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 42-42: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 47-47: Declare and assign separately to avoid masking return values.
(SC2155)
⏰ 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). (22)
- GitHub Check: cli-v1
- GitHub Check: stateless-js-v1
- GitHub Check: system-programs (sdk-libs, light-sdk-macros light-sdk light-program-test light-client batched-mer...
- GitHub Check: system-programs (sdk-anchor-test-program, ["cargo-test-sbf -p sdk-anchor-test", "cargo-test-sbf -...
- GitHub Check: system-programs (sdk-test-program, ["cargo-test-sbf -p sdk-native-test", "cargo-test-sbf -p sdk-v...
- GitHub Check: cli-v2
- GitHub Check: Test program-libs-slow
- GitHub Check: Test concurrent-merkle-tree
- GitHub Check: Test program-libs-fast
- GitHub Check: Test batched-merkle-tree-simulate
- GitHub Check: system-programs (random-e2e-test, ["cargo-test-sbf -p e2e-test"])
- GitHub Check: system-programs (system-cpi-test-v2-other, ["cargo-test-sbf -p system-cpi-v2-test -- --skip funct...
- GitHub Check: system-programs (system-cpi-test, ["cargo-test-sbf -p system-cpi-test", "cargo test -p light-syst...
- GitHub Check: system-programs (light-registry, ["cargo-test-sbf -p registry-test"])
- GitHub Check: system-programs (light-compressed-token, ["cargo-test-sbf -p compressed-token-test"])
- GitHub Check: system-programs (account-compression, ["cargo-test-sbf -p account-compression-test"])
- GitHub Check: system-programs (system-cpi-test-v2-event, ["cargo-test-sbf -p system-cpi-v2-test -- event::parse"])
- GitHub Check: system-programs (system-cpi-test-v2-functional, ["cargo-test-sbf -p system-cpi-v2-test -- functio...
- GitHub Check: system-programs (light-system-program, ["cargo-test-sbf -p system-test"])
- GitHub Check: Forester e2e test
- GitHub Check: stateless-js-v2
- GitHub Check: lint
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
program-libs/zero-copy/tests/derive/random.rs(1 hunks)scripts/create-release-pr.sh(1 hunks)scripts/detect-version-changes.sh(1 hunks)scripts/validate-packages.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- program-libs/zero-copy/tests/derive/random.rs
- scripts/validate-packages.sh
🧰 Additional context used
🪛 Shellcheck (0.11.0)
scripts/create-release-pr.sh
[warning] 50-50: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 51-51: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 52-52: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 57-57: Declare and assign separately to avoid masking return values.
(SC2155)
⏰ 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). (22)
- GitHub Check: system-programs (sdk-libs, light-sdk-macros light-sdk light-program-test light-client batched-mer...
- GitHub Check: system-programs (sdk-test-program, ["cargo-test-sbf -p sdk-native-test", "cargo-test-sbf -p sdk-v...
- GitHub Check: system-programs (sdk-anchor-test-program, ["cargo-test-sbf -p sdk-anchor-test", "cargo-test-sbf -...
- GitHub Check: Forester e2e test
- GitHub Check: cli-v1
- GitHub Check: stateless-js-v1
- GitHub Check: stateless-js-v2
- GitHub Check: system-programs (random-e2e-test, ["cargo-test-sbf -p e2e-test"])
- GitHub Check: system-programs (account-compression, ["cargo-test-sbf -p account-compression-test"])
- GitHub Check: system-programs (system-cpi-test-v2-functional, ["cargo-test-sbf -p system-cpi-v2-test -- functio...
- GitHub Check: system-programs (system-cpi-test, ["cargo-test-sbf -p system-cpi-test", "cargo test -p light-syst...
- GitHub Check: system-programs (light-registry, ["cargo-test-sbf -p registry-test"])
- GitHub Check: system-programs (system-cpi-test-v2-event, ["cargo-test-sbf -p system-cpi-v2-test -- event::parse"])
- GitHub Check: system-programs (system-cpi-test-v2-other, ["cargo-test-sbf -p system-cpi-v2-test -- --skip funct...
- GitHub Check: system-programs (light-compressed-token, ["cargo-test-sbf -p compressed-token-test"])
- GitHub Check: system-programs (light-system-program, ["cargo-test-sbf -p system-test"])
- GitHub Check: Test batched-merkle-tree-simulate
- GitHub Check: Test program-libs-slow
- GitHub Check: Test concurrent-merkle-tree
- GitHub Check: Test program-libs-fast
- GitHub Check: lint
- GitHub Check: cli-v2
| while IFS= read -r file; do | ||
| # Extract old and new version from the diff | ||
| local versions=$(git diff "${diff_args[@]}" -- "$file" | grep -E '^\+version|^-version' | grep -v '+++\|---') | ||
| local old_ver=$(echo "$versions" | grep '^-version' | head -1 | awk -F'"' '{print $2}') | ||
| local new_ver=$(echo "$versions" | grep '^\+version' | head -1 | awk -F'"' '{print $2}') | ||
|
|
||
| # Only process if version actually changed | ||
| if [ -n "$old_ver" ] && [ -n "$new_ver" ] && [ "$old_ver" != "$new_ver" ]; then | ||
| # Extract actual package name from Cargo.toml | ||
| local pkg_name=$(grep '^name = ' "$file" | head -1 | awk -F'"' '{print $2}') | ||
|
|
||
| if [ -n "$pkg_name" ]; then | ||
| echo "$pkg_name $old_ver $new_ver" | ||
| fi | ||
| fi | ||
| done < <(git diff "${diff_args[@]}" --name-only -- '**/Cargo.toml' | grep -E '(program-libs|sdk-libs|program-tests/merkle-tree)/') |
There was a problem hiding this comment.
Harden version parsing against non-version manifest edits.
set -euo pipefail means the git diff | grep … pipeline (Line 50) exits 1 whenever a touched Cargo.toml doesn’t change version. Because it sits in a command substitution, the whole create-release-pr.sh aborts before we ever see that the version bump is missing—precisely the case this script should catch and report. Add || true (or an explicit empty check) after the pipeline and likewise guard the subsequent grep extractions so we can continue when versions comes back empty instead of terminating.
A minimal fix mirrors the detection script:
- local versions=$(git diff "${diff_args[@]}" -- "$file" | grep -E '^\+version|^-version' | grep -v '+++\|---')
- local old_ver=$(echo "$versions" | grep '^-version' | head -1 | awk -F'"' '{print $2}')
- local new_ver=$(echo "$versions" | grep '^\+version' | head -1 | awk -F'"' '{print $2}')
+ local versions=$(git diff "${diff_args[@]}" -- "$file" \
+ | grep -E '^\+version|^-version' | grep -v '+++\|---' || true)
+ [ -n "$versions" ] || continue
+ local old_ver=$(echo "$versions" | grep '^-version' | head -1 | awk -F'"' '{print $2}' || true)
+ local new_ver=$(echo "$versions" | grep '^\+version' | head -1 | awk -F'"' '{print $2}' || true)📝 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.
| while IFS= read -r file; do | |
| # Extract old and new version from the diff | |
| local versions=$(git diff "${diff_args[@]}" -- "$file" | grep -E '^\+version|^-version' | grep -v '+++\|---') | |
| local old_ver=$(echo "$versions" | grep '^-version' | head -1 | awk -F'"' '{print $2}') | |
| local new_ver=$(echo "$versions" | grep '^\+version' | head -1 | awk -F'"' '{print $2}') | |
| # Only process if version actually changed | |
| if [ -n "$old_ver" ] && [ -n "$new_ver" ] && [ "$old_ver" != "$new_ver" ]; then | |
| # Extract actual package name from Cargo.toml | |
| local pkg_name=$(grep '^name = ' "$file" | head -1 | awk -F'"' '{print $2}') | |
| if [ -n "$pkg_name" ]; then | |
| echo "$pkg_name $old_ver $new_ver" | |
| fi | |
| fi | |
| done < <(git diff "${diff_args[@]}" --name-only -- '**/Cargo.toml' | grep -E '(program-libs|sdk-libs|program-tests/merkle-tree)/') | |
| while IFS= read -r file; do | |
| # Extract old and new version from the diff | |
| local versions=$(git diff "${diff_args[@]}" -- "$file" \ | |
| | grep -E '^\+version|^-version' | grep -v '+++\|---' || true) | |
| [ -n "$versions" ] || continue | |
| local old_ver=$(echo "$versions" | grep '^-version' | head -1 | awk -F'"' '{print $2}' || true) | |
| local new_ver=$(echo "$versions" | grep '^\+version' | head -1 | awk -F'"' '{print $2}' || true) | |
| # Only process if version actually changed | |
| if [ -n "$old_ver" ] && [ -n "$new_ver" ] && [ "$old_ver" != "$new_ver" ]; then | |
| # Extract actual package name from Cargo.toml | |
| local pkg_name=$(grep '^name = ' "$file" | head -1 | awk -F'"' '{print $2}') | |
| if [ -n "$pkg_name" ]; then | |
| echo "$pkg_name $old_ver $new_ver" | |
| fi | |
| fi | |
| done < <(git diff "${diff_args[@]}" --name-only -- '**/Cargo.toml' | grep -E '(program-libs|sdk-libs|program-tests/merkle-tree)/') |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 50-50: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 51-51: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 52-52: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 57-57: Declare and assign separately to avoid masking return values.
(SC2155)
🤖 Prompt for AI Agents
In scripts/create-release-pr.sh around lines 48 to 63, the command substitution
that captures changed version lines can exit non-zero under set -euo pipefail
when a Cargo.toml has no version changes, causing the script to abort; modify
the pipeline that builds `versions` to append `|| true` (or otherwise ensure it
never returns non-zero) and add an explicit empty check right after (if [ -z
"$versions" ]; then continue; fi) before running the subsequent grep/awk
extractions so those commands don’t fail on empty input and the loop simply
skips files without version changes.
| versions=$(git diff "${DIFF_ARGS[@]}" -- "$file" | grep -E '^\+version|^-version' | grep -v '+++\|---') | ||
| old_ver=$(echo "$versions" | grep '^-version' | head -1 | awk -F'"' '{print $2}') | ||
| new_ver=$(echo "$versions" | grep '^\+version' | head -1 | awk -F'"' '{print $2}') | ||
|
|
||
| # Only process if version actually changed | ||
| if [ -n "$old_ver" ] && [ -n "$new_ver" ] && [ "$old_ver" != "$new_ver" ]; then | ||
| # Extract actual package name from Cargo.toml | ||
| pkg_name=$(grep '^name = ' "$file" | head -1 | awk -F'"' '{print $2}') | ||
|
|
||
| if [ -n "$pkg_name" ]; then | ||
| PACKAGES+=("$pkg_name") | ||
| fi | ||
| fi |
There was a problem hiding this comment.
Allow non-version Cargo diffs without aborting.
With set -euo pipefail, the grep pipelines on Lines 35-37 exit with status 1 whenever a changed Cargo.toml lacks version edits (e.g., dependency bumps only). Because they're in command substitutions, the script terminates immediately, so any workspace touching a manifest without bumping version causes a false “no packages” failure. Guard the pipelines with || true (or restructure to test git diff output before grepping) so we safely skip those files instead of exiting.
Here’s one way to harden the block:
- versions=$(git diff "${DIFF_ARGS[@]}" -- "$file" | grep -E '^\+version|^-version' | grep -v '+++\|---')
- old_ver=$(echo "$versions" | grep '^-version' | head -1 | awk -F'"' '{print $2}')
- new_ver=$(echo "$versions" | grep '^\+version' | head -1 | awk -F'"' '{print $2}')
+ versions=$(git diff "${DIFF_ARGS[@]}" -- "$file" \
+ | grep -E '^\+version|^-version' | grep -v '+++\|---' || true)
+ [ -n "$versions" ] || continue
+ old_ver=$(echo "$versions" | grep '^-version' | head -1 | awk -F'"' '{print $2}' || true)
+ new_ver=$(echo "$versions" | grep '^\+version' | head -1 | awk -F'"' '{print $2}' || true)📝 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.
| versions=$(git diff "${DIFF_ARGS[@]}" -- "$file" | grep -E '^\+version|^-version' | grep -v '+++\|---') | |
| old_ver=$(echo "$versions" | grep '^-version' | head -1 | awk -F'"' '{print $2}') | |
| new_ver=$(echo "$versions" | grep '^\+version' | head -1 | awk -F'"' '{print $2}') | |
| # Only process if version actually changed | |
| if [ -n "$old_ver" ] && [ -n "$new_ver" ] && [ "$old_ver" != "$new_ver" ]; then | |
| # Extract actual package name from Cargo.toml | |
| pkg_name=$(grep '^name = ' "$file" | head -1 | awk -F'"' '{print $2}') | |
| if [ -n "$pkg_name" ]; then | |
| PACKAGES+=("$pkg_name") | |
| fi | |
| fi | |
| versions=$(git diff "${DIFF_ARGS[@]}" -- "$file" \ | |
| | grep -E '^\+version|^-version' | grep -v '+++\|---' || true) | |
| [ -n "$versions" ] || continue | |
| old_ver=$(echo "$versions" | grep '^-version' | head -1 | awk -F'"' '{print $2}' || true) | |
| new_ver=$(echo "$versions" | grep '^\+version' | head -1 | awk -F'"' '{print $2}' || true) | |
| # Only process if version actually changed | |
| if [ -n "$old_ver" ] && [ -n "$new_ver" ] && [ "$old_ver" != "$new_ver" ]; then | |
| # Extract actual package name from Cargo.toml | |
| pkg_name=$(grep '^name = ' "$file" | head -1 | awk -F'"' '{print $2}') | |
| if [ -n "$pkg_name" ]; then | |
| PACKAGES+=("$pkg_name") | |
| fi | |
| fi |
🤖 Prompt for AI Agents
In scripts/detect-version-changes.sh around lines 35 to 47, the command
substitutions calling grep/awk will exit with non‑zero under set -euo pipefail
when a changed Cargo.toml has no version edits; modify the command substitutions
to tolerate no matches by appending || true to the pipelines (or pre-check git
diff output before grepping) so the substitutions return an empty string instead
of failing the script, then keep the existing conditional that only proceeds
when old_ver and new_ver are non‑empty and different.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
program-tests/zero-copy-derive-test/tests/ui/pass/04_nested_vecs.rs (1)
1-55: Alignmutfeature usage between tests and library
The UI test (04_nested_vecs.rs) now usesZeroCopyMutunconditionally, but the derive macro and implementations inprogram-libs/zero-copy-deriveremain behind#[cfg(feature = "mut")](lib.rs:103, lib.rs:214, shared/mod.rs:6). This will break the build unless you
- re-add
cfg(feature = "mut")around the test,- include
"mut"in your default features in Cargo.toml, or- remove the
cfg(feature = "mut")guards in the library.program-tests/zero-copy-derive-test/tests/ui/pass/repr_c_packed_test.rs (1)
7-13: MakeZeroCopyMutderive available unconditionally
derive_zero_copy_mutis still guarded by#[cfg(feature = "mut")]; tests currently opt in via dependency features- to support unguarded
ZeroCopyMut, remove thecfg(feature = "mut")wrapper and add"mut"to the default features inCargo.tomlprogram-tests/zero-copy-derive-test/tests/random.rs (1)
315-316: Compile error:rng.gen()cannot produceOption<u64>.
Option<T>does not implementDistribution<Standard>. Build will fail here.Use explicit Some/None:
- compress_or_decompress_lamports: rng.gen(), + compress_or_decompress_lamports: if rng.gen() { Some(rng.gen()) } else { None },
♻️ Duplicate comments (1)
.github/workflows/rust.yml (1)
45-45: Critical: Fix cargo test invocation (previously flagged).This issue was already identified in a previous review but remains unfixed.
--testmust appear before--to tell cargo which integration test to run, not after (which passes it to the libtest harness and causes an error).Apply this diff:
- RUST_LOG=light_prover_client=debug cargo test -p batched-merkle-tree-tests -- --test test_simulate_transactions + RUST_LOG=light_prover_client=debug cargo test -p batched-merkle-tree-tests --test test_simulate_transactionsAnd at line 71:
- cargo test -p batched-merkle-tree-tests -- --test test_e2e + cargo test -p batched-merkle-tree-tests --test test_e2eAlso applies to: 71-71
🧹 Nitpick comments (5)
program-tests/zero-copy-derive-test/tests/ui/pass/13_large_struct.rs (1)
40-64: Formatting improvement enhances readability.The multi-line field initialization improves readability for this large struct with 25+ fields. The change is purely cosmetic with no impact on functionality.
program-tests/zero-copy-derive-test/tests/random.rs (3)
3-3: Module exposure is fine; consider removingpubfor minimal scope.
mod instruction_data;is sufficient inside this test crate. Keeping it non‑public avoids exporting unneeded symbols.
29-33: Imports look good; minor duplication can be trimmed.You also import
StdRng,Rng,SeedableRnginside test fns. Prefer one consistent import style to reduce noise.
589-599: Speed up tests and reduce log spam.10,000 iterations with per‑iteration
println!will slow CI and flood logs.Consider:
- Lower default iters and make it configurable.
- Gate prints behind an env var.
- let num_iters = 10_000; + let num_iters: usize = std::env::var("NUM_ITERS") + .ok() + .and_then(|s| s.parse().ok()) + .unwrap_or(1_000); @@ - println!("✓ Successfully tested InstructionDataInvokeCpi with {} inputs, {} outputs, {} new_addresses", - invoke_cpi_ref.input_compressed_accounts_with_merkle_context.len(), - invoke_cpi_ref.output_compressed_accounts.len(), - invoke_cpi_ref.new_address_params.len()); + if std::env::var_os("VERBOSE_TESTS").is_some() { + println!("✓ Successfully tested InstructionDataInvokeCpi with {} inputs, {} outputs, {} new_addresses", + invoke_cpi_ref.input_compressed_accounts_with_merkle_context.len(), + invoke_cpi_ref.output_compressed_accounts.len(), + invoke_cpi_ref.new_address_params.len()); + }You can apply the same gating to the similar print in the other test loop.
Also applies to: 630-634
program-tests/zero-copy-derive-test/tests/ui/fail/54_floating_point_types.stderr (1)
137-155: Reduce UI test brittleness; prefer pattern-based assertions or slim the case.The expanded diagnostics are verbose and likely to churn with rustc changes. Consider:
- Use pattern-based UI checks (compiletest
//~ ERROR ...orui_testpatterns) instead of full.stderr.- Or split/simplify the
.rsto focus on floats only, and move Vec/Option cases to separate tests to reduce duplicated errors.Example (optional) to slim 54_floating_point_types.rs to just floats:
use light_zero_copy_derive::{ZeroCopy, ZeroCopyMut}; #[derive(ZeroCopy, ZeroCopyMut)] #[repr(C)] pub struct FloatingPoint { - pub small: f32, - pub large: f64, - pub vec_f32: Vec<f32>, - pub vec_f64: Vec<f64>, - pub opt_f32: Option<f32>, - pub array_f64: [f64; 4], + pub small: f32, + pub large: f64, } fn main() {}If you're on compiletest, replacing
.stderrwith inline//~ ERROR:markers next to the offending lines will also make updates lighter.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (88)
.github/workflows/rust.yml(2 hunks)Cargo.toml(6 hunks)program-tests/batched-merkle-tree-test/Cargo.toml(1 hunks)program-tests/zero-copy-derive-test/Cargo.toml(1 hunks)program-tests/zero-copy-derive-test/src/lib.rs(1 hunks)program-tests/zero-copy-derive-test/tests/instruction_data.rs(1 hunks)program-tests/zero-copy-derive-test/tests/random.rs(2 hunks)program-tests/zero-copy-derive-test/tests/ui.rs(1 hunks)program-tests/zero-copy-derive-test/tests/ui/fail/01_empty_struct.rs(1 hunks)program-tests/zero-copy-derive-test/tests/ui/fail/07_tuple_struct.rs(1 hunks)program-tests/zero-copy-derive-test/tests/ui/fail/08_newtype_pattern.rs(1 hunks)program-tests/zero-copy-derive-test/tests/ui/fail/47_tuple_struct_empty.rs(1 hunks)program-tests/zero-copy-derive-test/tests/ui/fail/48_tuple_single_field.rs(1 hunks)program-tests/zero-copy-derive-test/tests/ui/fail/54_floating_point_types.rs(1 hunks)program-tests/zero-copy-derive-test/tests/ui/fail/54_floating_point_types.stderr(1 hunks)program-tests/zero-copy-derive-test/tests/ui/fail/55_usize_isize_types.rs(1 hunks)program-tests/zero-copy-derive-test/tests/ui/fail/55_usize_isize_types.stderr(2 hunks)program-tests/zero-copy-derive-test/tests/ui/fail/58_array_of_options.rs(1 hunks)program-tests/zero-copy-derive-test/tests/ui/fail/58_array_of_options.stderr(0 hunks)program-tests/zero-copy-derive-test/tests/ui/fail/67_char_type.rs(1 hunks)program-tests/zero-copy-derive-test/tests/ui/fail/67_char_type.stderr(2 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/02_single_u8_field.rs(1 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/03_all_primitives.rs(2 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/04_nested_vecs.rs(3 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/05_nested_options.rs(2 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/06_array_fields.rs(2 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/11_pubkey_fields.rs(2 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/12_mixed_visibility.rs(2 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/13_large_struct.rs(3 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/14_vec_of_arrays.rs(2 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/15_option_vec.rs(2 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/16_bool_fields.rs(2 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/17_signed_integers.rs(2 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/18_zero_sized_arrays.rs(2 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/19_max_sized_array.rs(1 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/20_nested_struct_fields.rs(2 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/21_enum_single_variant.rs(1 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/22_primitive_after_vec.rs(2 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/23_multiple_options_after_vec.rs(2 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/24_vec_option_vec.rs(2 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/25_all_optional.rs(3 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/26_deep_nesting.rs(2 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/27_mixed_arrays.rs(2 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/28_field_named_data.rs(2 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/29_field_named_bytes.rs(2 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/30_underscore_fields.rs(2 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/31_numeric_suffix_fields.rs(2 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/32_camel_case_fields.rs(3 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/33_single_letter_fields.rs(2 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/34_vec_of_bools.rs(2 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/35_option_bool.rs(2 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/36_array_of_bools.rs(3 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/37_meta_boundary_primitive.rs(4 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/38_meta_boundary_option.rs(2 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/39_enum_with_array.rs(1 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/40_enum_discriminant_order.rs(1 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/41_struct_with_lifetime_name.rs(2 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/42_reserved_keywords.rs(2 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/43_alternating_types.rs(2 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/44_all_vecs.rs(1 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/45_single_vec.rs(2 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/46_single_option.rs(3 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/49_max_meta_fields.rs(2 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/50_combination_all_features.rs(2 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/51_deep_nested_structs.rs(2 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/52_enum_containing_struct.rs(2 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/53_enum_containing_vec.rs(1 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/56_all_derives.rs(3 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/57_option_of_array.rs(3 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/59_vec_of_options.rs(2 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/60_option_pubkey.rs(1 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/61_vec_pubkey.rs(2 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/62_array_pubkey.rs(1 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/63_arrays_only.rs(1 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/64_option_first_field.rs(4 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/65_vec_of_vec.rs(2 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/66_triple_nested_option.rs(2 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/68_enum_containing_option.rs(1 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/69_very_long_field_names.rs(3 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/70_rust_type_field_names.rs(3 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/basic_enum.rs(1 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/basic_struct.rs(1 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/complex_enum.rs(2 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/repr_c_packed_test.rs(1 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/with_arrays.rs(1 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/with_options.rs(3 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/with_pubkey.rs(2 hunks)program-tests/zero-copy-derive-test/tests/ui/pass/with_vectors.rs(3 hunks)
💤 Files with no reviewable changes (1)
- program-tests/zero-copy-derive-test/tests/ui/fail/58_array_of_options.stderr
✅ Files skipped from review due to trivial changes (12)
- program-tests/zero-copy-derive-test/tests/ui/pass/52_enum_containing_struct.rs
- program-tests/zero-copy-derive-test/tests/ui/pass/28_field_named_data.rs
- program-tests/zero-copy-derive-test/tests/ui/pass/51_deep_nested_structs.rs
- program-tests/zero-copy-derive-test/tests/ui/pass/25_all_optional.rs
- program-tests/zero-copy-derive-test/src/lib.rs
- program-tests/zero-copy-derive-test/tests/ui/pass/66_triple_nested_option.rs
- program-tests/zero-copy-derive-test/Cargo.toml
- program-tests/zero-copy-derive-test/tests/ui/pass/with_options.rs
- program-tests/zero-copy-derive-test/tests/ui/pass/with_pubkey.rs
- program-tests/zero-copy-derive-test/tests/ui/pass/12_mixed_visibility.rs
- program-tests/zero-copy-derive-test/tests/ui/pass/26_deep_nesting.rs
- program-tests/zero-copy-derive-test/tests/ui/pass/22_primitive_after_vec.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- Cargo.toml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-14T00:36:53.191Z
Learnt from: ananas-block
PR: Lightprotocol/light-protocol#1909
File: program-libs/zero-copy/src/init_mut.rs:241-249
Timestamp: 2025-08-14T00:36:53.191Z
Learning: In the light-protocol zero-copy crate, performance is prioritized over safety checks for edge cases like Vec lengths exceeding u32::MAX, even when there might be wire format inconsistencies.
Applied to files:
program-tests/zero-copy-derive-test/tests/ui/pass/06_array_fields.rsprogram-tests/zero-copy-derive-test/tests/ui/pass/19_max_sized_array.rs
⏰ 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). (22)
- GitHub Check: system-programs (random-e2e-test, ["cargo-test-sbf -p e2e-test"])
- GitHub Check: system-programs (system-cpi-test-v2-functional, ["cargo-test-sbf -p system-cpi-v2-test -- functio...
- GitHub Check: system-programs (system-cpi-test-v2-other, ["cargo-test-sbf -p system-cpi-v2-test -- --skip funct...
- GitHub Check: system-programs (light-system-program, ["cargo-test-sbf -p system-test"])
- GitHub Check: system-programs (light-compressed-token, ["cargo-test-sbf -p compressed-token-test"])
- GitHub Check: system-programs (light-registry, ["cargo-test-sbf -p registry-test"])
- GitHub Check: system-programs (system-cpi-test, ["cargo-test-sbf -p system-cpi-test", "cargo test -p light-syst...
- GitHub Check: system-programs (account-compression, ["cargo-test-sbf -p account-compression-test"])
- GitHub Check: system-programs (system-cpi-test-v2-event, ["cargo-test-sbf -p system-cpi-v2-test -- event::parse"])
- GitHub Check: cli-v2
- GitHub Check: lint
- GitHub Check: cli-v1
- GitHub Check: stateless-js-v2
- GitHub Check: stateless-js-v1
- GitHub Check: system-programs (sdk-anchor-test-program, ["cargo-test-sbf -p sdk-anchor-test", "cargo-test-sbf -...
- GitHub Check: system-programs (sdk-test-program, ["cargo-test-sbf -p sdk-native-test", "cargo-test-sbf -p sdk-v...
- GitHub Check: system-programs (sdk-libs, light-sdk-macros light-sdk light-program-test light-client batched-mer...
- GitHub Check: Test program-libs-fast
- GitHub Check: Test program-libs-slow
- GitHub Check: Test concurrent-merkle-tree
- GitHub Check: Test batched-merkle-tree-simulate
- GitHub Check: Forester e2e test
🔇 Additional comments (79)
program-tests/zero-copy-derive-test/tests/ui/pass/46_single_option.rs (1)
15-32: Formatting-only cleanup looks good.
Struct literal, config init, andnew_zero_copycall remain semantically identical after reflow; no issues spotted.program-tests/zero-copy-derive-test/tests/ui/pass/43_alternating_types.rs (1)
48-49: Formatting looks good. Line-wrapping this call improves readability without changing behavior.program-tests/zero-copy-derive-test/tests/ui/pass/21_enum_single_variant.rs (1)
1-21: LGTM! Feature gate removal aligns with release flow standardization.The removal of the
#[cfg(feature = "mut")]gate makes this test compile unconditionally, which aligns with the broader changes across the UI test suite. The test logic correctly validates Borsh serialization compatibility with the ZeroCopy derive macro for a single-variant enum.program-tests/zero-copy-derive-test/tests/ui/pass/14_vec_of_arrays.rs (1)
2-2: LGTM! Formatting improvements enhance readability.The changes are purely cosmetic:
- Added blank lines for visual separation (lines 2, 28)
- Condensed struct initialization to single line (line 30)
- Split long function call for better readability (lines 34-35)
All logic remains unchanged and correct.
Also applies to: 28-28, 30-30, 34-35
program-tests/zero-copy-derive-test/tests/ui/pass/18_zero_sized_arrays.rs (1)
1-41: LGTM!The formatting changes (extra blank lines and line wrapping) are reasonable and improve readability without affecting the test logic. The test correctly validates the zero-sized array edge case for ZeroCopy trait implementations.
program-tests/zero-copy-derive-test/tests/ui/pass/16_bool_fields.rs (2)
2-2: LGTM!The removal of the
#[cfg(feature = "mut")]attribute enables this test to run unconditionally, which improves test coverage and aligns with the PR's broader pattern of removing feature gates from UI tests.
39-40: LGTM!The reformatted function call improves readability by splitting the long line. No functional changes.
program-tests/zero-copy-derive-test/tests/ui/pass/34_vec_of_bools.rs (1)
2-41: LGTM! Formatting improvements applied.The changes are purely formatting-related (whitespace adjustments, struct literal consolidation, and function call line breaks) and improve code readability. The removal of the
cfg(feature = "mut")gate mentioned in the summary aligns with the broader pattern of enabling unconditional test compilation across the test suite.program-tests/zero-copy-derive-test/tests/ui/pass/61_vec_pubkey.rs (2)
21-22: LGTM - formatting improvement.The vec initialization has been compacted to single-line format, which is more concise and equally readable for short vector literals.
2-2: LGTM - spacing adjustments.The blank line additions improve visual separation between logical sections of the test.
Also applies to: 28-28, 31-31, 35-35, 38-38
program-tests/zero-copy-derive-test/tests/ui/pass/64_option_first_field.rs (1)
2-57: LGTM! Formatting improvements enhance readability.The changes are purely cosmetic whitespace and formatting adjustments that improve code readability without altering functionality. The multiline formatting of the
new_zero_copyassignment (lines 43-44) makes the code easier to scan, and the additional empty lines provide better visual separation between test sections.program-tests/zero-copy-derive-test/tests/ui/pass/31_numeric_suffix_fields.rs (1)
2-2: LGTM!The formatting adjustments improve code readability without altering logic. The test correctly validates Borsh serialization and zero-copy operations for structs with numeric suffix fields.
Also applies to: 34-35, 39-39, 43-44
program-tests/zero-copy-derive-test/tests/ui/pass/40_enum_discriminant_order.rs (1)
1-31: LGTM!The test correctly validates enum discriminant handling for the zero-copy derive functionality. The logic is sound: it serializes an enum variant using Borsh, deserializes it using
zero_copy_at, and verifies that all bytes were consumed. The formatting change aligns with the PR's cleanup objectives.program-tests/zero-copy-derive-test/tests/ui/pass/20_nested_struct_fields.rs (1)
2-2: LGTM: Formatting changes only.The changes are purely cosmetic—an empty line adjustment and reformatting the struct construction to a single line. No functional or behavioral impact on the test.
Also applies to: 38-38
program-tests/zero-copy-derive-test/tests/ui.rs (1)
11-11: Good addition—verify that the newly enabled tests pass.Re-enabling the compile-fail tests improves coverage by validating error messages and compilation behavior. Ensure these tests pass in CI (i.e., fail compilation as expected).
program-tests/zero-copy-derive-test/tests/ui/pass/37_meta_boundary_primitive.rs (1)
2-2: LGTM! Formatting improvements enhance readability.The formatting changes (multi-line method calls, added spacing) improve code readability without altering functionality. The removal of the
cfg(feature = "mut")attribute mentioned in the summary makes this test unconditional, which aligns with the broader pattern across the PR to simplify zero-copy derive UI test gating.Also applies to: 14-14, 36-39, 41-41, 45-46, 57-57
program-tests/zero-copy-derive-test/tests/ui/pass/65_vec_of_vec.rs (1)
38-39: LGTM!Splitting this long line improves readability while preserving the logic.
program-tests/zero-copy-derive-test/tests/ui/pass/basic_struct.rs (1)
29-36: LGTM! Formatting improvements with no semantic changes.The added empty line and reformatted function call improve readability without affecting functionality. The test logic remains correct and thorough.
program-tests/zero-copy-derive-test/tests/ui/pass/59_vec_of_options.rs (1)
2-2: LGTM! Formatting improvements align with test consolidation effort.The removal of the
cfg(feature = "mut")gate (enabling unconditional compilation) and the reformatting of the function call improve readability and align with the broader test suite consolidation pattern described in the PR context.Also applies to: 43-44
program-tests/zero-copy-derive-test/tests/ui/pass/56_all_derives.rs (2)
2-2: Verify unconditional test execution is intended.The removal of the
cfg(feature = "mut")attribute means this test now runs unconditionally rather than being feature-gated. While this aligns with the broader PR pattern across multiple test files, ensure this change is intentional and that the test suite's behavior remains as expected.
46-47: LGTM!The line split improves readability without changing functionality.
program-tests/zero-copy-derive-test/tests/ui/pass/33_single_letter_fields.rs (1)
36-37: LGTM! Formatting improvements enhance readability.The formatting changes improve code readability by wrapping long function calls across multiple lines and simplifying the struct initialization to a more concise single-line form. These changes are purely cosmetic with no functional impact.
Also applies to: 39-39, 41-41, 45-46
program-tests/zero-copy-derive-test/tests/ui/fail/08_newtype_pattern.rs (2)
2-2: LGTM! Simplifies test configuration.Removing the
mutfeature gate makes the import unconditional, which simplifies the build and test configuration. The test's purpose (validating that the newtype pattern withVec<u8>fails) remains intact.
8-8: LGTM! Formatting improvement.Adding the trailing newline follows Unix text file conventions.
program-tests/zero-copy-derive-test/tests/ui/pass/complex_enum.rs (1)
1-22: LGTM! Unconditional compilation improves test coverage.Removing the
#[cfg(feature = "mut")]gate ensures this test runs in all build configurations, which is beneficial for catching regressions. The formatting adjustments are clean, and the test logic correctly validates ZeroCopy derive functionality with Borsh serialization.program-tests/zero-copy-derive-test/tests/ui/fail/55_usize_isize_types.rs (1)
1-15: LGTM: Feature gate removal enables unconditional test compilation.The removal of the
#[cfg(feature = "mut")]attribute allows this test to run unconditionally, which aligns with the broader PR pattern of removing feature gating across zero-copy derive tests. The test correctly validates that architecture-dependent types (usizeandisize) cannot implement theZeroCopytraits.program-tests/zero-copy-derive-test/tests/ui/fail/55_usize_isize_types.stderr (1)
137-155: LGTM: Test expectations updated to match current compiler output.The updated error diagnostics correctly reflect the compiler's behavior after removing the feature gate. The new error block (lines 137-155) provides additional context about the
isize: ZeroCopyNew<'_>bound issue and static size requirements, which improves test coverage.Also applies to: 408-412
program-tests/zero-copy-derive-test/tests/ui/pass/29_field_named_bytes.rs (1)
2-2: LGTM! Formatting improvements.The formatting changes improve readability without altering logic.
Also applies to: 30-30, 32-37, 44-44
.github/workflows/rust.yml (1)
63-63: LGTM! Test commands are correct.Both test invocations use proper syntax:
- Line 63: Standard package test.
- Line 65: Correctly places
--skipoptions after--as test harness arguments.Also applies to: 65-65
program-tests/zero-copy-derive-test/tests/ui/pass/57_option_of_array.rs (1)
2-2: LGTM: Formatting changes with no semantic impact.The multi-line formatting of
assert_eq!calls and thenew_zero_copyinvocation, along with minor whitespace adjustments, improve readability without altering the test logic.Also applies to: 28-39, 55-55, 60-61
program-tests/zero-copy-derive-test/tests/ui/pass/27_mixed_arrays.rs (1)
2-2: LGTM! Test cleanup and formatting improvements.The removal of the
cfg(feature = "mut")gate and formatting adjustments are appropriate. Running this test unconditionally improves coverage without introducing any functional changes.Also applies to: 33-33, 39-40
program-tests/zero-copy-derive-test/tests/ui/pass/60_option_pubkey.rs (1)
2-2: LGTM. Feature gate removal aligns with release flow improvements. Removal of#[cfg(feature = "mut")]simplifies the test and ensures both immutable and mutable zero-copy operations are always exercised. Please verify locally that this test compiles and passes without themutfeature gate.program-tests/zero-copy-derive-test/tests/ui/pass/68_enum_containing_option.rs (1)
1-26: LGTM! Unconditional compilation enabled.The removal of the
cfg(feature = "mut")gating allows this test to run in all configurations, improving test coverage. The formatting adjustment is appropriate.program-tests/zero-copy-derive-test/tests/ui/pass/basic_enum.rs (1)
1-21: LGTM! Feature gating removed and EOF formatting fixed.The removal of
cfg(feature = "mut")enables broader test execution, and the added trailing newline follows standard formatting conventions.program-tests/zero-copy-derive-test/tests/ui/pass/45_single_vec.rs (1)
1-48: LGTM! Formatting improved and feature gating removed.The changes enable unconditional compilation and consolidate the
SingleVecConfiginitialization for better readability. No semantic changes.program-tests/zero-copy-derive-test/tests/ui/pass/15_option_vec.rs (1)
1-47: LGTM! Formatting refined and feature gating removed.The removal of
cfg(feature = "mut")enables broader test coverage, and the formatting adjustments improve readability without altering behavior.program-tests/zero-copy-derive-test/tests/ui/pass/23_multiple_options_after_vec.rs (1)
1-58: LGTM! Line break improves readability.The feature gating removal enables unconditional test execution, and the multi-line formatting for
new_zero_copyimproves readability for this longer expression.program-tests/zero-copy-derive-test/tests/ui/pass/69_very_long_field_names.rs (1)
1-66: LGTM! Extensive formatting enhances readability for long identifiers.The removal of feature gating enables unconditional compilation, and the multi-line formatting significantly improves readability for the exceptionally long field names in this edge-case test.
program-tests/zero-copy-derive-test/tests/ui/pass/02_single_u8_field.rs (1)
1-35: LGTM! Feature gating removed.The removal of
cfg(feature = "mut")enables this test to run unconditionally, improving test coverage across configurations.program-tests/zero-copy-derive-test/tests/ui/pass/03_all_primitives.rs (1)
1-62: LGTM! Consistent formatting and feature gating removed.The changes enable unconditional compilation and improve readability by splitting the long
new_zero_copycall. The formatting adjustments maintain consistency with the test suite.program-tests/zero-copy-derive-test/tests/ui/pass/05_nested_options.rs (2)
2-4: LGTM! Import organization improves consistency.The reordering of Borsh imports to alphabetical order and the reorganization of zero-copy derive imports align with standard Rust conventions and improve code consistency across the test suite.
40-41: LGTM! Formatting improves readability.Wrapping the
new_zero_copycall across lines improves readability without changing behavior.program-tests/zero-copy-derive-test/tests/ui/pass/11_pubkey_fields.rs (1)
42-43: LGTM! Formatting improves readability.The line wrapping of the
new_zero_copycall is consistent with similar formatting adjustments across the test suite and enhances readability.program-tests/zero-copy-derive-test/tests/ui/pass/17_signed_integers.rs (2)
3-5: LGTM! Import organization improves consistency.The alphabetical reordering of imports follows Rust conventions and maintains consistency across the test suite.
39-40: LGTM! Formatting improves readability.Line wrapping of the
new_zero_copycall enhances readability and is consistent with similar changes throughout the test suite.program-tests/zero-copy-derive-test/tests/ui/pass/with_arrays.rs (1)
29-36: LGTM!The formatting improvements enhance readability. The empty line at Line 29 better separates test sections, and the line wrapping of the
new_zero_copycall at lines 35-36 improves code clarity.program-tests/zero-copy-derive-test/tests/ui/fail/01_empty_struct.rs (1)
2-2: LGTM!The removal of the feature gate (as described in the summary) and the resulting whitespace adjustment are part of the systematic refactoring across test files. The test logic remains intact and will now run unconditionally, which aligns with the PR's broader objectives.
program-tests/zero-copy-derive-test/tests/ui/pass/53_enum_containing_vec.rs (1)
2-2: Verify UI test runs unconditionally
Removal of the#![cfg(feature = "mut")]attribute makes this test compile and execute without feature gating; confirm it still passes in CI after this change.program-tests/zero-copy-derive-test/tests/ui/fail/07_tuple_struct.rs (1)
2-2: LGTM: imports are unconditional and consistent across all UI tests. No remainingcfg(feature = "mut")gates detected.program-tests/zero-copy-derive-test/tests/ui/pass/35_option_bool.rs (2)
36-37: LGTM! Formatting improves readability.The multi-line formatting for the
new_zero_copybinding makes the code more readable without changing behavior.
3-5: LGTM: import reordering and derive imports improve clarity
The alphabetical sorting of Borsh imports and explicit import ofZeroCopy/ZeroCopyMutderive macros make dependencies clearer, and removing thecfg(feature = "mut")gate aligns this test with unconditional CI execution.Please verify locally that
program-tests/zero-copy-derive-test/tests/ui/pass/35_option_bool.rscompiles and passes without the feature gate.program-tests/zero-copy-derive-test/tests/ui/pass/63_arrays_only.rs (1)
2-2: LGTM! Feature gate removal aligns with CI test inclusion.The removal of the
cfg(feature = "mut")attribute enables this test to compile unconditionally, which aligns with the PR's goal of updating CI matrices to include new tests. The test exercises both read-only and mutable zero-copy operations, and its unconditional inclusion is consistent with the broader pattern of feature gate removals across test files.program-tests/zero-copy-derive-test/tests/ui/pass/41_struct_with_lifetime_name.rs (1)
2-2: Verify test compiles without the "mut" feature
teeisn’t available in CI; rerun with:#!/bin/bash set -e cd program-tests/zero-copy-derive-test if cargo build --tests --no-default-features &> /tmp/build_output.txt; then echo "=== COMPILATION SUCCESSFUL ===" else echo "=== COMPILATION ERRORS FOUND ===" cat /tmp/build_output.txt exit 1 fiprogram-tests/zero-copy-derive-test/tests/ui/pass/49_max_meta_fields.rs (2)
2-2: LGTM! Formatting improvements enhance readability.The blank lines, single-line config initialization, and reflowed function call all improve code clarity.
Also applies to: 59-66
1-90: Verify CI invocation of UI tests
No references to49_max_meta_fields.rswere found in CI configs nor any remaining feature gates in the file; please confirm your CI workflows still execute theprogram-tests/zero-copy-derive-test/tests/uisuite so this test runs as intended.program-tests/zero-copy-derive-test/tests/ui/pass/39_enum_with_array.rs (1)
1-23: LGTM — manual test verification requiredRemoving
cfg(feature = "mut")makes this UI test unconditional and aligns with the PR’s simplification goals. The test correctly validates enum zero-copy with Borsh compatibility. Manually confirm that the39_enum_with_arrayUI test passes without the feature gate.program-tests/zero-copy-derive-test/tests/ui/pass/13_large_struct.rs (2)
84-85: Consistent multi-line formatting.Splitting the
new_zero_copyinvocation across multiple lines maintains consistency with the struct initialization formatting above.
2-4: LGTM: Import refactoring aligns with release flow objectives.The migration to the external
light_zero_copy_derivecrate is correct, andlight-zero-copy-deriveis declared in Cargo.toml.program-tests/zero-copy-derive-test/tests/ui/pass/19_max_sized_array.rs (1)
2-2: Confirmed availability ofZeroCopyMutderive macro andZeroCopyAtMuttrait without the “mut” feature. Removing#[cfg(feature = "mut")]is safe.program-tests/zero-copy-derive-test/tests/ui/pass/36_array_of_bools.rs (1)
16-16: LGTM! Formatting improvements enhance readability.The inline initialization on line 16 is more concise and idiomatic. The multi-line split on lines 32-33 improves readability without changing behavior.
Also applies to: 32-33
program-tests/zero-copy-derive-test/tests/ui/pass/24_vec_option_vec.rs (2)
42-43: LGTM!The formatting improvement enhances readability by breaking a long line into two lines, following Rust conventions.
2-2: Approve feature gate removal
The CI workflows and test crate dependencies always enable themutfeature, so removing thecfg(feature = "mut")guard is safe and redundant.program-tests/zero-copy-derive-test/tests/ui/pass/with_vectors.rs (1)
40-41: LGTM — formatting improvements and unconditional test execution.The multi-line let binding split improves readability, and whitespace adjustments (lines 30, 54) are appropriate. Removing the
#[cfg(feature = "mut")]attribute ensures this UI test runs unconditionally, aligning with the PR’s release-validation goals. Unable to auto-run the test in this environment—please manually confirm that thewith_vectorsUI test compiles and passes without the feature gate.program-tests/zero-copy-derive-test/tests/ui/pass/32_camel_case_fields.rs (1)
8-8: Removal of explicit#[cfg(feature = "mut")]is safe. The test crate’s Cargo.toml enables themutfeature, soZeroCopyMutandzero_copy_at_mutremain available.program-tests/zero-copy-derive-test/tests/ui/pass/62_array_pubkey.rs (1)
2-2: LGTM — feature gate removal is applied consistently across the zero-copy test suite. BothZeroCopyMutderive and mutable operations now run unconditionally in all tests.program-tests/zero-copy-derive-test/tests/ui/fail/67_char_type.rs (1)
3-12: Derive import looks goodImporting both derives and dropping the feature gate keeps this failure test aligned with the new macro location while ensuring it always runs. No issues spotted.
program-tests/zero-copy-derive-test/tests/ui/fail/58_array_of_options.rs (1)
3-11: Consistent derive usageThanks for un-gating this test and pulling derives from
light_zero_copy_derive; everything matches the new setup.program-tests/zero-copy-derive-test/tests/ui/fail/67_char_type.stderr (1)
210-233: Updated stderr matches new derive diagnosticsThe additional
ZeroCopyNew/ZeroCopyTraitsfailures line up with running both derives without the feature gate. Snapshot looks accurate.Also applies to: 696-705
program-tests/zero-copy-derive-test/tests/ui/pass/repr_c_packed_test.rs (2)
2-2: Blank line added for readability.The blank line improves code formatting after removing the feature gate.
22-41: LGTM! Test logic is well-structured.The test correctly validates both packed and aligned struct zero-copy operations with proper assertions for remaining bytes.
program-tests/zero-copy-derive-test/tests/ui/pass/44_all_vecs.rs (2)
3-68: LGTM! Comprehensive test coverage.The test logic is well-structured and thoroughly validates the zero-copy implementation for structs with all Vec fields:
- Borsh serialization/deserialization
- Immutable zero-copy access
- Mutable zero-copy access
- Byte length calculations
- Manual field manipulation and verification
2-2: Approve removal of#[cfg(feature = "mut")]: themutfeature is enabled by default in this test crate via thelight-zero-copydev-dependency.program-tests/zero-copy-derive-test/tests/ui/fail/47_tuple_struct_empty.rs (1)
8-8: LGTM!Adding a trailing newline after the final function is good practice and improves file formatting consistency.
program-tests/zero-copy-derive-test/tests/ui/pass/50_combination_all_features.rs (1)
12-39: Ensure themutfeature is enabled for this UI test
DerivingZeroCopyMutand importingZeroCopyAtMutrequires themutfeature. Confirm that thezero-copy-derive-testcrate’s default features includemut, or re-add the#[cfg(feature = "mut")]guard (or enable the feature in the test harness). Building with--no-default-featuresotherwise fails.program-tests/zero-copy-derive-test/tests/instruction_data.rs (1)
12-25: Cosmetic derive reordering only.Moving
Defaultwithin the derive list is non‑functional and fine.program-tests/zero-copy-derive-test/tests/ui/pass/70_rust_type_field_names.rs (2)
44-45: LGTM: Multiline formatting improves readability.The let binding has been reformatted to span multiple lines, which improves readability without changing behavior.
1-64: Removal of file-levelcfg(feature = "mut")is safe. Tests pull inmuttransitively via thelight-zero-copydev-dependency (features = ["derive","mut","std"]), no other UI tests contain acfg(feature = "mut")gate, and the crate builds successfully.program-tests/zero-copy-derive-test/tests/ui/fail/48_tuple_single_field.rs (1)
2-2: LGTM; same un-gating consideration as other UI tests.Import is now unconditional; verify the feature exposing
ZeroCopyMutis enabled in test/dev builds to avoid parse failures.program-tests/zero-copy-derive-test/tests/ui/fail/54_floating_point_types.rs (1)
2-2: Enable themutfeature on the derive crate in tests.
In program-tests/zero-copy-derive-test/Cargo.toml, update thelight-zero-copy-derivedev-dependency:[dev-dependencies] -light-zero-copy-derive = { workspace = true } +light-zero-copy-derive = { workspace = true, features = ["mut"] }Likely an incorrect or invalid review comment.
program-tests/zero-copy-derive-test/tests/ui/fail/47_tuple_struct_empty.rs
Show resolved
Hide resolved
| #![cfg(feature = "mut")] | ||
|
|
||
| use borsh::{BorshDeserialize, BorshSerialize}; | ||
| use light_zero_copy::traits::{ZeroCopyAt, ZeroCopyAtMut, ZeroCopyNew}; |
There was a problem hiding this comment.
Restore the mut feature gate.
This UI test still depends on ZeroCopyAtMut/ZeroCopyMut, which remain behind the mut feature in the crate. Without the crate-level #![cfg(feature = "mut")] guard, default (featureless) compiletest runs now hit missing imports/derives and fail to build. Please reintroduce the gate so the pass test only runs when the mut feature is available.
Apply this diff to put the guard back:
+#![cfg(feature = "mut")]
+
// Edge case: Fields with names close to reserved keywords📝 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.
| use light_zero_copy::traits::{ZeroCopyAt, ZeroCopyAtMut, ZeroCopyNew}; | |
| #![cfg(feature = "mut")] | |
| // Edge case: Fields with names close to reserved keywords | |
| use light_zero_copy::traits::{ZeroCopyAt, ZeroCopyAtMut, ZeroCopyNew}; | |
| ... |
🤖 Prompt for AI Agents
In program-tests/zero-copy-derive-test/tests/ui/pass/42_reserved_keywords.rs
around line 4, the test requires the crate-level `mut` feature but the file no
longer has the `#![cfg(feature = "mut")]` gate; add the crate-level attribute
`#![cfg(feature = "mut")]` at the top of the file (before any `use` or item
declarations) so the test only compiles when the `mut` feature is enabled,
ensuring imports/derives (ZeroCopyAtMut/ZeroCopyMut) are available.
8472053 to
d6c8b4b
Compare
d6c8b4b to
2b38c86
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/rust.yml (1)
31-31: Job skipped on push events due to PR-onlyifconditionOn push,
github.event.pull_requestis null, so the job won’t run. Allow pushes to run while keeping draft PRs skipped.Apply:
- if: github.event.pull_request.draft == false + if: github.event_name != 'pull_request' || github.event.pull_request.draft == false
♻️ Duplicate comments (1)
.github/workflows/rust.yml (1)
45-45: Fix cargo--testplacement (passes to libtest instead of cargo)
--testmust be before the final--so cargo selects the integration test target. Current form will fail. This was flagged earlier.Apply:
- RUST_LOG=light_prover_client=debug cargo test -p batched-merkle-tree-test -- --test test_simulate_transactions + RUST_LOG=light_prover_client=debug cargo test -p batched-merkle-tree-test --test test_simulate_transactions- cargo test -p batched-merkle-tree-test -- --test test_e2e + cargo test -p batched-merkle-tree-test --test test_e2eAlso applies to: 71-71
🧹 Nitpick comments (1)
.github/workflows/rust.yml (1)
13-19: Keep path filters consistent for PRs (include.cargo/**)PR trigger misses
.cargo/**, so changes to.cargo/config*won’t run CI on PRs, only on push. Align with push filters.Apply:
pull_request: branches: ["*"] paths: - "program-libs/**" - "**/Cargo.*" + - ".cargo/**" - "prover/client/**" - ".github/workflows/rust.yml"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/rust.yml(2 hunks).github/workflows/sdk-tests.yml(1 hunks)program-libs/zero-copy-derive/src/lib.rs(7 hunks)program-tests/zero-copy-derive-test/Cargo.toml(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- program-libs/zero-copy-derive/src/lib.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- program-tests/zero-copy-derive-test/Cargo.toml
- .github/workflows/sdk-tests.yml
⏰ 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). (22)
- GitHub Check: stateless-js-v1
- GitHub Check: cli-v1
- GitHub Check: system-programs (system-cpi-test-v2-functional, ["cargo-test-sbf -p system-cpi-v2-test -- functio...
- GitHub Check: system-programs (light-system-program, ["cargo-test-sbf -p system-test"])
- GitHub Check: system-programs (system-cpi-test-v2-event, ["cargo-test-sbf -p system-cpi-v2-test -- event::parse"])
- GitHub Check: system-programs (random-e2e-test, ["cargo-test-sbf -p e2e-test"])
- GitHub Check: system-programs (system-cpi-test-v2-other, ["cargo-test-sbf -p system-cpi-v2-test -- --skip funct...
- GitHub Check: system-programs (account-compression, ["cargo-test-sbf -p account-compression-test"])
- GitHub Check: system-programs (system-cpi-test, ["cargo-test-sbf -p system-cpi-test", "cargo test -p light-syst...
- GitHub Check: system-programs (light-registry, ["cargo-test-sbf -p registry-test"])
- GitHub Check: system-programs (light-compressed-token, ["cargo-test-sbf -p compressed-token-test"])
- GitHub Check: Forester e2e test
- GitHub Check: lint
- GitHub Check: system-programs (sdk-libs, light-sdk-macros light-sdk light-program-test light-client batched-mer...
- GitHub Check: system-programs (sdk-anchor-test-program, ["cargo-test-sbf -p sdk-anchor-test", "cargo-test-sbf -...
- GitHub Check: system-programs (sdk-test-program, ["cargo-test-sbf -p sdk-native-test", "cargo-test-sbf -p sdk-v...
- GitHub Check: cli-v2
- GitHub Check: stateless-js-v2
- GitHub Check: Test program-libs-slow
- GitHub Check: Test program-libs-fast
- GitHub Check: Test concurrent-merkle-tree
- GitHub Check: Test batched-merkle-tree-simulate
🔇 Additional comments (1)
.github/workflows/rust.yml (1)
76-85: No action needed for Redis tag –redis:8.0.1is confirmed valid on Docker Hub.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
Cargo.toml (2)
236-253: Reconsiderverify = falsein cargo-release configDisabling verify risks publishing broken crates if external validation misses something. Either:
- Set
verify = true, or- Keep it
falsebut ensure the release workflow blocks publish on full build/test and a dry-run publish.Safer default:
-verify = false +verify = trueIf you keep
verify = false, ensure release-publish workflow gates on:
- Full
cargo build --workspace --all-featurescargo test --workspacecargo publish --dry-runfor each publishable crate- Optional consumer-check build without
[patch.crates-io]
162-162: Internal versions verified; clarifylight-program-profilerdependency
- All path dependencies’ versions match their crate manifests.
- If
light-program-profileris a workspace crate, useworkspace = true(or explicitpath); otherwise confirm it’s intended as an external dependency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
Cargo.toml(6 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). (22)
- GitHub Check: system-programs (system-cpi-test-v2-other, ["cargo-test-sbf -p system-cpi-v2-test -- --skip funct...
- GitHub Check: system-programs (light-system-program, ["cargo-test-sbf -p system-test"])
- GitHub Check: system-programs (account-compression, ["cargo-test-sbf -p account-compression-test"])
- GitHub Check: system-programs (system-cpi-test, ["cargo-test-sbf -p system-cpi-test", "cargo test -p light-syst...
- GitHub Check: system-programs (random-e2e-test, ["cargo-test-sbf -p e2e-test"])
- GitHub Check: system-programs (system-cpi-test-v2-event, ["cargo-test-sbf -p system-cpi-v2-test -- event::parse"])
- GitHub Check: system-programs (light-registry, ["cargo-test-sbf -p registry-test"])
- GitHub Check: system-programs (light-compressed-token, ["cargo-test-sbf -p compressed-token-test"])
- GitHub Check: system-programs (system-cpi-test-v2-functional, ["cargo-test-sbf -p system-cpi-v2-test -- functio...
- GitHub Check: system-programs (sdk-test-program, ["cargo-test-sbf -p sdk-native-test", "cargo-test-sbf -p sdk-v...
- GitHub Check: system-programs (sdk-libs, light-sdk-macros light-sdk light-program-test light-client batched-mer...
- GitHub Check: system-programs (sdk-anchor-test-program, ["cargo-test-sbf -p sdk-anchor-test", "cargo-test-sbf -...
- GitHub Check: stateless-js-v2
- GitHub Check: cli-v2
- GitHub Check: cli-v1
- GitHub Check: stateless-js-v1
- GitHub Check: lint
- GitHub Check: Forester e2e test
- GitHub Check: Test program-libs-slow
- GitHub Check: Test concurrent-merkle-tree
- GitHub Check: Test batched-merkle-tree-simulate
- GitHub Check: Test program-libs-fast
🔇 Additional comments (1)
Cargo.toml (1)
232-232: Gate fork-only code behind a non-default feature and add isolated publish CI job
- Verify that
mem-profiling(referenced inprogram-libs/macros/src/lib.rs) is not included in default features.- Add a CI step building each publishable crate standalone—without the
[patch]override.- Document in CONTRIBUTING how to enable the fork for local profiling.
fd2b24a to
10122e5
Compare
10122e5 to
8763917
Compare
Summary by CodeRabbit
New Features
New Tools
Tests
Chores