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.
...
- 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
...