Things To Look For In A Code Review
Tags: codereview, programming, tutorial, productivity
Published: 2023-07-09 07:02:29

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](https://bigtechdigest.substack.com/) 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](https://adr.github.io/).
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**

- [Google’s Code Review Practices](https://google.github.io/eng-practices/review/reviewer/looking-for.html)
- [What To Look For In A Code Review by Jetbrains](https://blog.jetbrains.com/upsource/2015/07/23/what-to-look-for-in-a-code-review/)