DEV Community

Cover image for Things To Look For In A Code Review
Big Tech Digest
Big Tech Digest

Posted on

Things To Look For In A Code Review

Hi folks 👋! Big Tech Digest is evolving. On top of the standard newsletter with the latest Big Tech and startups engineering blog post links sent to you twice a month, I will periodically write entries like this, delving into various aspects of Software Engineering that will help you build and design better systems.

The first entry in this series focuses on code reviews. Over the years, I’ve been putting together a checklist I always refer to when performing a code review. The items in this list led to significant improvements and notably enhanced the quality of what would be merged into the main branch on many occasions in the past. Furthermore, I recommend that code authors also use this list before submitting a Pull Request to expedite the review process.

I encourage you to start using the checklist today. Go further and standardise the code review process within your team or organisation by using this list as an inspiration.


💡 Subscribe to the official newsletter here to receive one email every two weeks!


General

  1. ⭐️ Make sure the code works as expected. Give it a local/dry run and test it manually.
    1. Make sure the logic works as expected.
  2. ⭐️ Make sure the code is consistent with the rest of the code base. Check if it meets the team’s/company’s standards and conventions.
  3. ⭐️ Ask the code author to highlight the main changes and focus your attention there.
  4. ⭐️ Assess if the code is based on assumptions.
    1. Ask the code author to validate them before merging.
  5. ⭐️ Identify new code conventions that can be derived from the Pull Request. If so, propose an ADR.
  6. ⭐️ Make sure the code fits in the overall architecture.
  7. ⭐️ Ensure all expected business errors are handled in a reasonable way.
  8. ⭐️ Check if the code/solution is over-engineered.
  9. ⭐️ Run Code Inspect in your IDE (e.g. IntelliJ).
  10. ⭐️ Look for compiler/linter warnings.
  11. ⭐️ Check if the code is easy to understand, read, and reason about.
  12. ⭐️ Improve naming.
  13. ⭐️ Check if logging is added where necessary.
    1. Check if all errors/exceptions are logged.
    2. Consider adding warning logs where necessary.
  14. ⭐️ Ensure tech and business metrics are being collected.
  15. ⭐️ Make sure alerting is added with reasonable thresholds if necessary.
  16. ⭐️ Make sure the code is extendable/evolvable where needed.
  17. ⭐️ Ensure the code is easily testable.
  18. ⭐️ Check if validation is added where necessary.
    1. Propose the alternatives for validation error handling if there are any.
  19. ⭐️ Assess if it’s worth adding that new library/dependency.
  20. ⭐️ Check if optimising the code is possible while maintaining all other mentioned requirements.
  21. ⭐️ Check if the code is placed in reasonable packages/directories/modules.
  22. ⭐️ Assess if there is any performance penalty for introducing the new code.
  23. ⭐️ Check if error log messages are understandable and carry enough information.
  24. ⭐️ Ensure the necessary documentation is added.
  25. ⭐️ Check if the order of arguments is consistent across the classes/layers.
  26. ⭐️ Keep security in mind and look for potential vulnerabilities. Try to abuse the new code.
  27. ⭐️ Make sure the proposed time-outs are reasonable.
  28. ⭐️ Ensure retries are added where necessary.
  29. ⭐️ Propose adding a circuit breaker added if required.
  30. ⭐️ Look for DB query optimisations.

Testing

  1. ⭐️ Ensure automated tests exist (unit, integration, acceptance).
  2. ⭐️ Run code coverage to find missing tests.
  3. ⭐️ Ensure tests are easy to read and understand.
  4. ⭐️ Ensure test names are easy to read and understand.
  5. ⭐️ Make sure tests are checking the actual requirements.
  6. ⭐️ Ensure all edge cases are tested.
  7. ⭐️ Check if new tests are slow and propose to optimise them, if possible.
  8. ⭐️ Ensure that the E2E/integration tests added in this PR are really needed.
    1. Understand the test performance penalty of adding those.
  9. ⭐️ Run tests many times to check if they’re flaky.
  10. ⭐️ Ask yourself if some of the tests could be removed.

Best code review practices for code authors

  • 👉 Strive to get your PR reviewed as early as possible.
  • 👉 Ask for feedback early, ideally before start writing the code. Remember: the cost of addressing PR comments is higher than consulting the proposed solution before writing the code.
  • 👉 Highlight the most crucial change(s) in the PR so that the code reviewer can focus their attention on that part during the review.

Best code review practices for code reviewers

  • 👉 Balance “perfect” vs “good enough”. Striving for perfection often makes PR reviews slow and painful.
  • 👉 Balance “personal preference” vs “underlying principles”. Base your comments on “underlying principles” and established/agreed team standards, not personal preference unless strongly justified.

Thanks for reading! Let me know in the comments what else you look for in a code review!

Resources

Top comments (0)