Checklist

Are all requirements implemented?
Single responsibility of a single PR. No mixed changes.
Is the code change the best way to implement the feature?
Are there any regression issues introduced in the existing features?
If it’s a breaking change, is it well communicated?
Is the code change complete?
1. Documents updated
2. Migration included
3. Configurations updated
4. Other systems updated/notified
5. Infrastructure(Database, etc.)
6. Non-functional requirements
  1. Metrics
  2. Tracing
  3. Logging
Are there any major issues found?
1. Performance
2. Security
3. Thread safe
4. Edge cases
Are errors are gracefully handled?
Are there sufficient level of unit tests/e2e tests?
Is the code change generally readable?
1. Can I get an understanding of the desired behavior just by doing quick scans through unit and acceptance tests?
2. Do the methods do what the name of the method claims that they’ll do? Same for classes?
3. Method is too long
4. Method has too many parameters
5. Deeply nested if/else controls
Are general coding best practices followed?
1. No hard coded stuff
2. No duplications
3. Separate of concerns
4. Easy to extend
5. Comments
6. Naming convention followed

General Rules