What to Look for in Code Reviews

A detailed guide on what to focus on as a reviewer during pull requests.

Jul 25, 2025

Hi Friends,

Welcome to the 132nd issue of the Polymathic Engineer.

Code reviews are one of the most valuable practices in software development. A good code review catches bugs before they reach production, improves code quality, and helps team members learn from each other. On the other side, a poor review wastes time and misses critical issues.

As a reviewer, one of the main challenges is knowing what to focus on. When looking at a pull request with many changes, it's easy to focus on the wrong things. Should you spend time checking syntax? Look for potential bugs? Focus on code style? All these are important, but in the right priority.

Having a structured approach instead of randomly scanning through code changes makes the process more effective and efficient. This issue walks through the key areas to focus on during code reviews, starting with the most important considerations and working down to the details.

Cut Code Review Time & Bugs in Half

Code reviews are critical but time-consuming. CodeRabbit acts as your AI co-pilot, providing instant Code review comments and potential impacts of every pull request.

Beyond just flagging issues, CodeRabbit provides one-click fix suggestions and lets you define custom code quality rules using AST Grep patterns, catching subtle issues that traditional static analysis tools might miss.

CodeRabbit has so far reviewed more than 10 million PRs, installed on 1 million repositories, and used by 70 thousand Open-source projects. CodeRabbit is free for all open-source repo's.

Get Started Today

Thanks to the CodeRabbit team for collaborating with me to this newsletter issue.

1. Design Considerations

Design review represents the most important aspect of a code review because design problems are the hardest and most expensive to fix later. When evaluating the design, you're basically asking whether the proposed changes make the overall system better or worse.

When you check if the code is well-designed for the system, you have to look beyond the immediate usefulness and see how the change fits into the architecture as a whole. Does the new code follow patterns that are already there in your codebase? Does it add new concepts that are at odds with abstractions that are already in place? If you make a good change, it should feel like it fits in with the current system instead of being a strange addition.

Also, think about whether the code belongs where it is. Sometimes, developers add features to existing classes or modules simply because it's easier to do so, even when it would be better suited for a different part of the codebase or even a separate library.

Code that changes parts of the user interface elements doesn't belong in a data access layer. Business logic shouldn't be scattered across utility classes. Ask yourself: Would a new developer know where to look to change this feature in six months if they came along? If the answer is no, the code might be in the wrong place.

2. Functionality Review

After confirming the design makes sense, the next step is checking if the code works the way it should. This means looking for bugs that automated tests might miss and thinking about what could go wrong.

First, figure out what the developer intended the code to do. Is that goal achieved? Implementation doesn't always match the stated goal or bring side effects that were not planned for. Look at the code with fresh eyes and see if it all makes sense.

Pay special attention to concurrency issues if the code does parallel processing. Race conditions and deadlocks are difficult to catch through testing alone because they depend on specific timing conditions. You need to think through the logic carefully.

Resource leaks are another problem that you should be on the lookout for. Does the code correctly close files, database connections, or sockets? Is there any situation where exceptions might stop the cleanup code from running? These problems often don't show symptoms until the system is under load.

Finally, think whether you need to validate the code's behavior yourself. If the change affects user-facing functionality, especially UI changes, it's often worth asking for a demo or trying the feature yourself. Reading code isn't always enough to figure out how something works.

3. Complexity

Complex code is expensive. It's harder to change safely, takes longer to understand, and is more likely to have bugs.

The key question to ask is whether another developer could easily understand and change this code. If you can't follow the logic during a code review, other developers will likely struggle with it afterward.

Check complexity at different levels. Is a single line too complex? Is a function trying to do too much? A function that handles user input validation, database queries, and email sending all at once violates the single responsibility principle and is challenging to test and maintain.

Over-engineering is a type of complexity to be aware of. This includes code that is too generic or adds features that are not yet needed. A typical example is creating abstract base classes and interfaces for something that has only one implementation and no clear need for multiple ones.

When you abstract too soon, you often get the wrong abstraction, which is harder to work with than none at all.

Test Quality

Tests are an integral part of the code, and they need the same level of attention during reviews. Good tests catch bugs and give confidence when making changes. Bad tests waste time and create a false sense of security.

First, check if the right types of tests are included. Does the change need unit tests, integration tests, or end-to-end tests? Tests should always be in the same pull request, unless they are handling an emergency.

Look at whether the tests actually check the right things. Will they fail when the code is broken? It's surprisingly common to see tests that always pass, even when the code has serious bugs. Run through the test logic mentally and make sure it's validating the behavior it claims to test.

Check if the tests make explicit, simple assertions. A test that checks twelve different things in one method is hard to understand when it fails. Each test should focus on a specific behavior and clearly indicate what went wrong if it fails.

Watch out for tests that depend too much on implementation details. If changing how something works internally requires rewriting all the tests, such tests aren't providing much value. Good tests look at behavior and results, not at specific steps for execution.

Naming and Documentation

Good names make code self-documenting. Bad names force the readers to look up implementation details to figure out what something does.

Look for names that communicate what something is or does. A variable called data or info doesn't tell anything useful. A variable called userAccountBalance is clearer about what it contains.

Names shouldn't be too long or too short. They should be the right length. Make sure that the names follow the rules your team has set up. New code shouldn't use snake case if the rest of your code uses camel case for variables. Code is easy to read when it is consistent.

When reading comments, pay attention to whether they show why they explain why something exists, not what it does. Comments that repeat what the code already says are useless. Comments that explain the reasoning behind a decision are valuable.

Look for comments that indicate the code is too complex. If you need a paragraph to explain what five lines of code do, those five lines should be simplified or broken into smaller, clearer pieces.

Don't forget about documentation that lives outside the code. If the change affects how people build, test, or release the system, make sure that the documentation is updated to reflect that. Nothing is more frustrating than following outdated setup instructions.

6. Style and Consistency

Style matters because it affects how quickly developers can read and understand code. When code follows consistent patterns, developers don't have to decipher formatting, but can focus on the logic instead.

Check that the code follows your team's style guide. Most teams have set rules about indentation, bracket placement, line length, and other formatting details. The rules are there to keep the codebase consistent, not to make things difficult for developers.

If you want to suggest a style improvement that isn't covered in your style guide, mark it as a nit to show it's not mandatory. Don't block a pull request because you prefer a different way of formatting code. Personal tastes shouldn't get in the way of getting work done.

However, consistency is more than just the way things are formatted. How variables are named, how errors are handled, and how code is organized should be consistent throughout the entire codebase. The code is easier to understand and work with when coders stick to the same patterns.