Starting a new code review process, trying to make it simple. After searching the web among my old links (No More Hacks), I’m sumarizing here my conclusion.

Review/Checklist guidelines (for short review, focused on one topic):

  • specific to the review : avoid having a generic checklist to long
  • Measurable point : ensure the decision is black or white, avoiding conflicting decision
  • Agreed among the team : ensure guidelines/checklist have team agreement
  • time box approach : avoid endless meeting, do them short/focused, we’ll try a 30minutes approach per topic.
  • No more than 6  items : 30 minutes is short … to many item is not realistic
  • Nothing automated : everything that can be automated, must be automated. To use human brain for that.
  • updated on a regular basis : review, team skill, application issues, change over time, so review and update your checklist to match team priority.

Having that in minds, a quick meeting among the team help us to define 3 checklist : Java, Unit test, static code analysis.

Java :

  • Documentation : does the reviewer can easily understand the code
  • Correct handling of try/catch code
  • justify visibility of method (private/public/….)
  • Test policy of the code described

Unit test :

  • does all bugs fixed are illustrated by a unit test ?
  • does mock objects are used
  • does tests are deterministic and don’t required external dependencies

Static code analysis tool

  • Focus on top priority issues : memory leak and JDBC connection management
  • For each fix, ensure a test exist to ensure behavior is kept. If you change behavior ensure impact at application level (add functional tests)
  • Investigate to ensure it’s not a false/positive, if you decide it’s a false positive, make it reviewed by someone else.