Tuesday, 3 January 2017

Code Reviews 3 - As an Author, When am I ready for review?

This checklist for the author calling the review. The checklist helps avoid code reviews that just talk about the trivial stuff.

As an author, I should respect the time that my reviewers are putting into my submission. In that case, I wish to have my work as complete as possible and have a clear list of inputs ready for the reviewer, so as to get the most effective code review possible and not waste any ones time.

Authors's Code Review Checklist

  1. Tests complete and passing
  2. Code style adhered to
  3. Code complete
  4. Programmer documentation is complete
  5. Code, Test and Developer Documentation is committed
  6. All static analysis run and passing

Exploration

  1. Tests complete and passing. All legacy unit and integration tests are running and passing. Prove that our changes have no undesired side-effects on existing functionality. Any new tests that are relevant are written, running and passing. We apply the same source control standards to test code.
  2. Code style adhered to. I'm not saying that you have to use the same IDE as everyone else, but you should match the style that the majority of the code is written in as this enhances readability. So configure your IDE or run Checkstyle to ensure compliance.
  3. Code complete. A no brainer, obvious? Well the code should be complete enough to make sense, run and do something useful. It might not be the complete solution. The author should be pragmatic.
  4. Programmer documentation is complete. If you are writing a library, the API, javadoc and sample usages in the SDK are all complete.
  5. Code, Test and Developer Documentation is committed. Change management 101. Programmers share via source control. Typically it should be committed on a side branch, to be merged into main or master once the review is complete. The use of source control helps enforce discipline. It's obvious if the author changes the contents of the code up for review and helps prevent confusion for the reviewer.
  6. All static analysis run and passing. The reviewer shouldn't be looking for stuff that static analysers should be finding. You should have run PMD, findbugs and/or sonarqube with all pre-agreed rule sets.

Once I have completed all six parts, I am ready to submit my code review to reviewers.

Supporting resources

Clean Code, Robert C. Martin
The art of readable code, Boswell and Foucher
Pragmatic Programmer, Hunt and Thomas
A Guide for Code Reviews
Code Review Matters and Manners
Peer Reviews in software, Karl Wiegers
What to look for in a code review, Trisha Gee

What makes a good check in?

Updated 04/01/2016

Updated 13/04/2017

Updated 23/09/2017

Happy new year 2017

Welcome to 2017! 2016 was a busy year. I spoke a little on programming, code review and a fair bit on teams, agile and communication.

2017 will continue the theme. I'll focus on production of software and more on pairing and code reviews. Looking forward to it!