| name | my-code-review |
| description | Code review principles and checklist for reviewing any codebase regardless of language. Use when the user asks to review code, perform a code review, assess a pull request, or evaluate code quality across design, tests, performance, security, and correctness. |
Code Review
Based on "What to look for in a code review" by Trisha Gee (Java Champion, JetBrains) and personal notes
When to use
- User asks to review code or a pull request
- User asks for a code review checklist
- User wants to assess the quality of a change
- User mentions design, readability, test coverage, security, or correctness concerns
Overview
A structured, language-agnostic checklist to guide thorough code reviews. Reviews should reduce cognitive load, catch correctness issues, and share knowledge — not just find bugs.
Rules
Design
- Architecture fit — Does the change align with the overall architecture and existing conventions?
- SOLID — Are Single Responsibility, Open/Closed, Liskov, Interface Segregation, and Dependency Inversion respected?
- DDD — Are domain concepts clearly named and properly modelled?
- YAGNI / KISS — No speculative generality; prefer the simplest solution that works
- Code reuse — Is there a refactoring or extraction opportunity to avoid duplication?
Readability & Maintainability
- Understandability — Can a new reader understand the intent without context?
- Naming — Are names precise, consistent, and self-documenting?
- Happy path vs exceptional cases — Are both paths handled explicitly and clearly?
- Configuration vs hard-coded values — No magic constants; prefer named config
Tests
- Coverage of new code — Are all new branches and edge cases covered?
- Test intent — Does each test name and assertion communicate why, not just what?
- Tests are code — Apply the same quality rules (naming, readability, no duplication)
- Granularity — Right level: unit > integration > end-to-end; avoid testing implementation details
- Edge cases — Cardinality (empty, single, many), nullability, boundary values, error paths
- Limitations — Are test limitations intentional and documented, or accidental gaps?
- Performance & security tests — Are they needed? Are they present?
- Pair review option — Reviewers can write missing tests as part of the review
Performance
- Requirements — Does the implementation meet the stated performance requirements?
- Readability vs performance trade-off — Only optimise when measurements justify it
- Network cost — Are batching and call counts considered?
- Resource management — Are connections, streams, and handles properly closed?
- Memory leaks — Are data lifecycle and collection bounds controlled?
- Locks & race conditions — Are shared resources properly protected?
- Concurrency vs parallelism — Is the right model applied?
- Pool configuration — Use safe defaults; only tune with evidence
Data Structures
- Right choice — Is the data structure appropriate for the access pattern and complexity (Big-O)?
- Pitfalls — Watch for lazy evaluation traps, infinite streams, or iterator invalidation
- Optionality — Is absence of a value modelled explicitly (Option/Maybe/Optional) rather than null?
Security
- Automated checks — Are dependency scanners (e.g. Dependabot) and SAST tools in CI?
- Dependency surface — Are new dependencies justified and minimal?
- Regulatory requirements — Does the change touch data subject to compliance rules (GDPR, PCI, …)?
Correctness
- Wrong data structure — Could the structure cause subtle bugs (e.g. set vs list, map ordering)?
- Race conditions — Is concurrent access safe?
- Caching — Are cache invalidation and staleness handled correctly?
Cross-cutting Concerns
- Documentation impact — Does the change require updating docs, ADRs, or READMEs?
- UI / error messages — Are user-visible messages clear, actionable, and tested?
- Automated vs human review split — Delegate formatting/style to linters; focus human review on design and logic
Culture
- Share tooling: IDE configs, linter rules, and CI checks should be committed and consistent
- Reviewers should ask "Have you thought about…?" for security, edge cases, and docs — not just critique
- Prefer collaborative tone; a review is a knowledge-sharing session, not an audit
