Tuesday, April 29, 2014

Steps of a Code Review

Pair programming is better than code reviews, but sometimes you don't have a lot of choice and a review is your best option.  In such a circumstance, how should you go about it?

In my current context, we only do code reviews if another team owns the development, and we can't (or aren't ready to) enforce our coding standards.  That means we can't insist it was TDD'd, or even that it has tests.

  1. Does it work?
  2. Names & commit message
    • Does it have a commit message that describes what was changed?
    • Do the methods tell you what they do?
    • Do the variable names tell you what they're used for.
  3. Tests
    • Are there tests where appropriate?
    • Are there enough tests.
    • Is the test code clean with one assertion per test?
    • Is use of mocking appropriate?
  4. Style
    • Are methods shorter than 10 (or so) lines?
    • Are classes short?
You'd want to reject code for 1 no matter what.  Initially we'll reject it if 2 doesn't work as well.

We probably want to slowly start enforcing 3 and 4.