Code Review Guidelines | Focus on Logic, Not Style
Review correctness, design, and security—let automation handle formatting. Complete reviews in 15-60 minutes. Ask questions, explain why, praise good code.
5 min read
Code reviews catch bugs, share knowledge, and improve code quality. Good reviews are collaborative and educational. Bad reviews are slow, nit-picky, or rubber-stamped. This guide covers effective code review practices.
Review Focus Areas
| Area | Priority |
|---|---|
| Correctness | High - Does it work? |
| Design | High - Is it maintainable? |
| Edge cases | Medium - What can break? |
| Style | Low - Let linters handle |
Review Process
Submitting Reviews
CODE REVIEW PROCESS
═══════════════════
BEFORE SUBMITTING PR:
─────────────────────────────────────
Author should:
├── Self-review first
├── Run tests locally
├── Check linting passes
├── Write clear description
├── Link to issue/ticket
├── Keep PR small (< 400 lines ideal)
└── Prepared PR
PR DESCRIPTION:
─────────────────────────────────────
Include:
├── What: Summary of changes
├── Why: Context and motivation
├── How: Approach taken
├── Testing: How it was tested
├── Screenshots: If UI change
└── Complete context
REVIEWER SELECTION:
─────────────────────────────────────
├── Domain expert
├── Someone unfamiliar (fresh eyes)
├── Not always same person
├── Rotate reviewers
├── Balance load
└── Right reviewers
REVIEW TIMELINE:
─────────────────────────────────────
├── Request review → Within 24 hours
├── Feedback addressed → Same day if small
├── Not blocking for days
├── Priority attention
└── Fast feedback loop
Giving Reviews
Effective Feedback
GIVING REVIEWS
══════════════
WHAT TO LOOK FOR:
─────────────────────────────────────
High priority:
├── Correctness: Does it work?
├── Edge cases: What could break?
├── Security: Any vulnerabilities?
├── Design: Is it maintainable?
├── Testing: Are tests adequate?
└── Important issues
Lower priority:
├── Performance (unless critical path)
├── Minor style issues
├── Nit-picks
├── Personal preferences
└── Let automation handle
HOW TO GIVE FEEDBACK:
─────────────────────────────────────
Ask questions:
├── "What happens if X is null?"
├── "Did you consider approach Y?"
├── "Could this be simplified?"
└── Collaborative tone
Explain why:
├── Not: "Use X instead of Y"
├── But: "X would be clearer here because..."
├── Understanding, not orders
└── Educational
Label feedback:
├── [Must fix]: Blocking issues
├── [Suggestion]: Optional improvements
├── [Nit]: Very minor, non-blocking
├── [Question]: Seeking understanding
└── Clear expectations
GOOD FEEDBACK EXAMPLE:
─────────────────────────────────────
"[Suggestion] Consider extracting this
logic into a separate function—it would
make testing easier and the main function
more readable. Happy to discuss if you
see it differently!"
BAD FEEDBACK:
─────────────────────────────────────
├── "Wrong"
├── "Don't do this"
├── "???"
├── Unhelpful, unclear
└── Avoid these
PRAISE GOOD CODE:
─────────────────────────────────────
├── "Nice refactor here!"
├── "Great test coverage"
├── "Clever solution"
├── Recognition matters
└── Not just criticism
Review Culture
Healthy Dynamics
REVIEW CULTURE
══════════════
PRINCIPLES:
─────────────────────────────────────
├── Review code, not people
├── Assume good intent
├── Collaborative, not adversarial
├── Learning opportunity
├── Everyone's code gets reviewed
├── Including seniors
└── Healthy culture
AVOID:
─────────────────────────────────────
├── Nit-picking minor issues
├── Demanding style preferences
├── Blocking for trivial reasons
├── Being rude or condescending
├── Rubber-stamping (no review)
├── Taking criticism personally
└── Anti-patterns
RESOLVE DISAGREEMENTS:
─────────────────────────────────────
├── Discuss synchronously if stuck
├── Focus on code, not ego
├── Escalate if needed
├── Accept when you're wrong
├── Move on
└── Professional resolution
AUTOMATION:
─────────────────────────────────────
Let tools handle:
├── Formatting (Prettier, Black)
├── Linting (ESLint, Pylint)
├── Type checking
├── Test running
├── Security scanning
├── Humans focus on logic
└── Efficient use of time
GitScrum Integration
Linking Reviews to Work
GITSCRUM FOR REVIEWS
════════════════════
TASK LINKING:
─────────────────────────────────────
├── Link PR to GitScrum task
├── Update task status
├── Traceability
├── What shipped when
└── Connected workflow
DEFINITION OF DONE:
─────────────────────────────────────
DoD includes:
├── ☐ PR submitted
├── ☐ Code reviewed
├── ☐ Feedback addressed
├── ☐ Approved
├── ☐ Merged
└── Review required
REVIEW METRICS:
─────────────────────────────────────
Track:
├── Time to review
├── Review depth
├── Revision cycles
├── Identify bottlenecks
└── Process improvement
Best Practices
For Code Reviews
Anti-Patterns
REVIEW MISTAKES:
✗ Taking days to review
✗ Giant PRs
✗ Rubber-stamping
✗ Nitpicking style
✗ Condescending comments
✗ No explanations
✗ Blocking trivially
✗ Never praising