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

Browser not supported