This document serves as a guide to reviewers to keep different areas in mind when reviewing a PR. This will increase consistency between reviewers.
Readability / Coding Best Practices
- Software should be easy to understand. The code inputs and outputs, what each line of the code is doing, should be relatively easy to discern.
- Code should be properly formatted. This is enforced by eslint during the CI build.
- Ensure proper naming conventions are being used (proper casing). This is enforced by eslint during the CI build.
- Good, descriptive naming should be used for variables, functions, methods, classes etc.
- No hard coding is used unless absolutely necessary.
Maintainability / Architecture
- The code should follow any defined architecture (such as HTML, Javascript, CSS in appropriate files).
- Code should align with existing code patterns/technologies.
- Use appropriate design patterns.
- Code should not be written in technologies which need to be phased out unless absolutely necessary.
- Generally accepted or predefined security practices should have been followed.
- Logs are being generated such that code can be maintainable.
- Database migrations should follow the DatabaseMigrations guidelines.
Testing / Reliability
- There should be appropriate level of tests written.
- Tests should make sense and should cover all important areas.
- Tests should have been run, and test data should be provided if needed.
- Any relevant data needed to run the tests should be provided.
- Exceptions should be appropriately handled.
- Appropriate response codes are being returned.
Speed & Performance
- Consider performance implications and resource consumption of the code.
- Database queries should be returning only the relevant data.
- Query performance should be considered.
Documentation
- Ensure that documentation was created or updated appropriately.
- Documentation should be clear and easy to understand.