Code review 7 - Code Reviewers Checklist

So you've just agreed to be a code reviewer for another developers work. What should I check is available before the review can go ahead.

1. The problem specification
2. Any supporting documentation or knowledge
3. Test reports
4. Clearly outlined key tests
5. Static analysis reports
6. Source code or artefact ready for review is committed in source control

The author should provide these clearly to the reviewer. Note they do not all apply to all situations and all artefacts, so a pragmatic approach needs to be taken. It is important not to waste the reviewers' time by making them trawl through large amounts of information. The author must make it easy for the reviewer to provide feedback quickly. This might mean supplying documents or web pages or a short whiteboard session between the author and the reviewers to set the context for the review.

The Problem Specification

Why does this artefact/change exist? What problem is it supposed to solve? This is the primary question we should answer. Obviously we are looking for other things like code readability, code quality etc but does it actually do what it's supposed to do?

Supporting knowledge or documentation

What's the context of this artefact that is up for review. The quickest way to transfer this to the reviewer maybe via a quick face to face session.

Test Reports

Does this change or artefact break existing functionality? Test reports of regression should prove that the changes do not impact legacy.

Clear Tests

Take a look at the tests that are accompanying the artefact/change. Do they conform to industry standard like Given When Then? Do the tests reflect the language of the problem? Are they correctly coupled? Are they Good tests?

Static analysis

There are some fantastic tools out there like PMD, findbugs and Sonar. Is your project using any of them? Has the artefact for review passed all static analyses checks? For any failures, are there good, objective reasons why we are violating the rule?

Committed into source control

There are few things more annoying than code passed around via email, slack or instant chat. This is mainly because it rapidly becomes impossible to keep track of what that code currently does. One changed line can have a profound impact on the result. There's just no debate about it, use a source control system. Git combined with googles Gerrit system is very good.