docs: add mega-evm review checks to REVIEW.md#352
Conversation
🧬 Mutation testingNo results at |
🧬 Mutation testing — ✅ PASSNothing to test — no mutants were generated on the changed lines. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ed86042417
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| - `no_std` compatibility must be maintained in the `mega-evm` crate — no direct `std::` usage. | ||
| Follow the existing pattern: `#[cfg(not(feature = "std"))] use alloc as std;`. | ||
| - New workspace dependencies should use `default-features = false` — features are opted-in explicitly. | ||
| - **Release-blocking dependencies.** A crate that gets published must not depend on an unpublished crate; `cargo publish -p <crate> --locked` has to resolve at release time. Flag a new dependency that would break the next release's resolution. |
There was a problem hiding this comment.
This added Markdown bullet puts multiple sentences on the same physical line, so it violates the repository’s Markdown convention and makes future edits inconsistent with the surrounding documentation style; split the bold label and each following sentence onto separate lines.
AGENTS.md reference: AGENTS.md:L347-L348
Useful? React with 👍 / 👎.
| - **Benchmark methodology.** A perf-comparison PR must pin comparable hardforks on both arms (a hard-coded `OpSpecId` that ignores the enum default silently benches the wrong fork), use an even round count (odd defeats A/B alternation), and adjudicate quantitatively. Don't drop a `saturating_sub`/guard "for speed" without a measured, attributable gain. | ||
| - **Storage-layout parity.** Where Rust mirrors a Solidity storage layout, add a parity test against that layout so a field reordering fails a test instead of drifting silently into a consensus bug. |
There was a problem hiding this comment.
These added Markdown bullets also keep the bold label and following prose on the same line, with the benchmark bullet packing several sentences together; please wrap each sentence onto its own line to satisfy the repo’s documentation convention.
AGENTS.md reference: AGENTS.md:L347-L348
Useful? React with 👍 / 👎.
vincent-k2026
added
spec:unchanged
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
- Summary: Doc-only PR adding three review-guidance bullets to REVIEW.md. Content matches the PR description and no existing rule is altered.
- Inline comments: 2 minor stylistic findings on the sentence-per-line convention.
- Body-only items: none.
Pre-mortem: no verifiable high-impact failure path found.
Still open from earlier reviews: none.
| - `no_std` compatibility must be maintained in the `mega-evm` crate — no direct `std::` usage. | ||
| Follow the existing pattern: `#[cfg(not(feature = "std"))] use alloc as std;`. | ||
| - New workspace dependencies should use `default-features = false` — features are opted-in explicitly. | ||
| - **Release-blocking dependencies.** A crate that gets published must not depend on an unpublished crate; `cargo publish -p <crate> --locked` has to resolve at release time. Flag a new dependency that would break the next release's resolution. |
There was a problem hiding this comment.
[Minor] These new bullets pack the bold header and multiple sentences on one line, breaking the CLAUDE.md convention (One sentence, one line) and the pattern already used by existing REVIEW.md entries (see lines 14-16, 25-27, 44-47): bold header on its own line, then each subsequent sentence on its own indented continuation line.
| Their comments — especially the `file:line:col` mutation-location references — must be kept up to date when the referenced source moves. | ||
| Flag any PR that shifts lines in a mutated source file but leaves a now-stale location reference (or an orphaned/renamed test) in `tests/mutation/`; a reference that no longer points at its mutant defeats the purpose of the linkage. | ||
| These tests should not be hand-edited otherwise — see `crates/mega-evm/tests/mutation/main.rs`. | ||
| - **Benchmark methodology.** A perf-comparison PR must pin comparable hardforks on both arms (a hard-coded `OpSpecId` that ignores the enum default silently benches the wrong fork), use an even round count (odd defeats A/B alternation), and adjudicate quantitatively. Don't drop a `saturating_sub`/guard "for speed" without a measured, attributable gain. |
There was a problem hiding this comment.
[Minor] Same convention issue as line 37: multiple sentences on one line. Split the bold header and each sentence onto its own line.
1 participant
Supplements the baseline rubric with mega-evm-specific checks it doesn't cover:
cargo publish -p <crate> --lockedmust resolve at release time).Additive only — no existing rules changed.