This checklist should be used by code reviewers to guide their feedback and by review authors in understanding what reviews will be looking for.  This is a living document and it should be updated by the Team Leads in a continual effort to improve our code base by improving our process.  Considerable effort should be made to keep this document concise and easy to work with; if a checklist item may be simplified or even removed by automation, the project should prioritize this to realize gains across our development community.

Clarity

• Proper spelling
• Conforms to Style Guide
• Class, method & variable names are clear and concise
• Test method names should clearly describe what is being tested and under what conditions (eg. shouldNotAllowXWhenYIsActive)
• Class / method is not unreasonably long or complex
• Gets the job done - and no more.  Extraneous types or concepts aren’t introduced because we think we might need them in the future

• Dead code, useless comments, TODO’s, debug logging has been removed

Documentation

• Javadoc exists for all public interfaces (Package, Class, Method)
• RESTful interfaces are well documented in RAML. Every parameter and return has a JSON schema. JSON schemas are defined in their own JSON files and put in the schemas subfolder.
  
• Documentation explains why something exists over how it works
• Documentation covers edge cases and lays out the contract - success, error, exception.
• README introduces the purpose of the services and guides developers in getting started.

Encapsulation

• The package demarcations are useful - if package private scope is used, the contents of the package are still properly encapsulated
• Classes and methods have private scope, unless they need to be package-private or public
• Good OOD is used - message passing over get().get().get().
• Interfaces expose exactly what’s needed, and no more.  e.g. db internals
• Service boundaries are respected
• Types are used appropriately.  e.g. use UUID not integer,  Integer instead of String
• Util classes are not used – in OOP, object should contain both data and a behavior performed over that data. Regular classes should be used instead or behavior should be added directly to the already existing classes as non-static methods.

Resiliency

• Unit tests exist and they adequately cover edge cases
• Integration tests exist and they adequately test the interface contract (i.e. for RAML we have JSON schema's for parameters and returns)
• All tests that are meant to verify searching or filtering, do so on a larger dataset than just what matches the search criteria (e.g. an IT that searches user by username should run with at least a few users with different usernames in the database)
• Proper error handling techniques are used
• Exceptions are only used when there is a programming error
• External dependencies are brought in with care - is it mature, is it stable, does its functionality overlap with an existing piece
• Log messages are useful in a service oriented context and utilize an appropriate level

• All automated tests run & pass, tests aren’t removed unless appropriate

Security

• Actions that need to be secured, are.
• Rate limiting is considered

Reference

Reasoning & Example:  http://blog.fogcreek.com/increase-defect-detection-with-our-code-review-checklist-example/

Motech's checklist:  docs.motechproject.org/en/latest/development/code_review.html