Code Review Strategies

2022-10-06

Reviewing merge requests is hard, especially when you don't have a plan or good habits for reviewing code. I tend to approach most PRs at work in a haphazard way. At a given time I may be more likely to focus in on one aspect or another.

My goal with these notes is to document some of the different "angles" or "lenses" for reviewing code.

Angles🔗

• Meta
  • Correct issue number
  • Changes are expected / product approved
  • Changes are scheduled and merged within window for release
• Typos
• Formatting
• Logic
  • Look at loops and conditionals for mistakes
• Behaviour
  • Product oriented
  • What do the changes actually do
• Codebase health
  • Do the changes fit well within existing codebase
  • Are the changes "hacky"
  • Unit tests are added where possible
• Performance
  • Lookout for data access or other expensive operations in loops or func that may be called many times
• Architecture / Abstractions
  • Does the new code have any architecture "smells"
  • Is architecture or abstraction missing
  • Is there too much architecture or abstraction
  • Keep it simple
• Domain Specific
  • Feature flags are used
  • Data access not added to the engine