Code Review Guidelines

Rust PR reviews balance soundness, API design, and operability. Reviewers prioritize panics, unsafe, async blocking, error mapping, and dependency changes.

Recipe

Reviewers scan in this order:

1. PR description links ticket and explains why
2. CI green (fmt, clippy, test, locked build)
3. Diff size reasonable for topic
4. Tests cover behavior change
5. Security-sensitive paths (auth, SQL, FFI)

When to reach for this:

• Every PR before approval
• Training new reviewers
• Calibrating nitpick vs blocker

Working Example

Blockers (request changes)

// Blocking DB in async handler
async fn handler() {
    std::thread::sleep(Duration::from_secs(1)); // BLOCKER
}
 
// unwrap in library code
pub fn parse(s: &str) -> User {
    serde_json::from_str(s).unwrap() // BLOCKER in domain crate
}
 
// unsafe without SAFETY comment
unsafe { ptr.read() } // BLOCKER

Important (should fix)

• New dependency without cargo deny / team approval
• Cargo.lock missing on binary crate dep bump
• Missing #[instrument] on new async service boundary
• Clone on large struct in loop without justification

Nit (optional)

• Naming bikeshedding when rustfmt/clippy clean
• Micro-opts without profile data

Deep Dive

Review Checklist

Tone

Ask questions ("Could this lock across await?") over accusations. Link to cookbook sections.

Gotchas

• Rubber-stamping green CI - logic bugs still slip. Fix: read test assertions.
• Style-only review - misses soundness. Fix: follow priority order above.
• Blocking on preference - team fatigue. Fix: encode in clippy/rustfmt.
• Ignoring PR size - 2000-line drive-by. Fix: request split.
• Missing security lens on small diffs - auth bypass in one line. Fix: always check auth paths.

Alternatives

FAQs

Related

• Conventions & style guide
• Unsafe Review Skill
• Git best practices
• 50 Rust rules

Stack versions: This page was written for Rust 1.97.0 (edition 2024), Tokio 1.x, Axum 0.8, serde 1.0, sqlx 0.8, clap 4, and Polars 0.46+.