Code Review Best Practices

Chapter 2 · Practice 2 of 6

Check that the change does what the author intended, including edge cases

From What to look for in a review

Why it matters

Ask whether the change does what the author meant it to do, and whether that is good for its users: both end users and the developers who will call this code.

Think through what tests often miss:

  • Edge cases: empty input, very large input, missing data, the first and last item
  • Concurrency: two requests at once, race conditions, deadlocks. These are hard to spot by running the code, so read carefully.
  • Failure: what happens when a dependency is down or slow?
  • User facing changes: for UI changes, look at a screenshot or run the branch. The guide notes that it is hard to judge UI from code alone.

If you cannot easily reason about whether some concurrent code is correct, that is itself a finding: ask for a simpler approach.