I’m a firm believer in the practice of code reviews. When done correctly it can significantly improve the quality of software and is among the most effective knowledge-sharing tools in a team. In the previous article I shared a few tips how to create a great pull request. Following them as a team will make everyone’s life easier, so keep them in mind at all times. In this article, I’ll focus on effectively reviewing PRs.

TL;DR — Be nice! Your attitude and approach will be mirrored by your peers when they are reviewing your pull requests! Remember the common goal of your team – to achieve business value while writing great code you will be proud of.

1. Read the ticket before reading the code

Reviewing a PR without checking the associated ticket is like preparing for an exam by reading a random book picked up from the library. The text in the PR might or might not be relevant to the task at hand. You want to be sure it is.

Start the code review by reading the associated work item first. Understand what is required and build a high-level mental picture how this can be achieved. Form a rough expectation for the code that will follow. If you have no clue how to solve the issue / implement the feature at this stage, treat this as a red flag to yourself. Chances are your review won’t be as in-depth as needed. If you feel completely in the wild – ask another team member to review this as well.

2. Scan the list of changed files

Quickly go through the list of changed files and compare it with your mental model. If you were expecting 3 changed files and instead there are 30, something is wrong. It could be a violation of some of the tips for great PRs, could be that your expectation is way off, but also it could be a general issue with the approach taken to solve the problem.

If you were expecting some UI changes but you see the core files of the project updated, something is wrong. Usually that’s a sign of misunderstanding in the ticket – the expectations laid out, its scope or a missed “detail”.

If you see a file that’s not related to any of the rest – something might be wrong. Often that’s a sign of git management issues – perhaps leftovers from merging, wrong rebasing, incorrect conflict solving, etc.

3. Review code, not style

As preparation for next steps, remember that you should focus on the code and the logic behind it, not its style. Although consistent code style is important for a repository, this is not the best way to enforce it. Instead it should be driven by a strictly defined style guide and these checks should be fully automated. If done properly, the question of style shouldn’t come up often in pull requests (apart from proposals to amend the style guide). For JVM-based projects, Checkstyle and Detekt are great static analysis tools commonly used together to ensure a certain style guide.

4. Start with the tests

If applicable, start by reading the tests (unit / integration / end-to-end). They are your shortcut to understand what the new code is doing and whether the pull request achieves its task. A set of clear, isolated and targeted unit tests is a great foundation of the changes. It hints good overall quality. Test names should sound somewhat similar to the acceptance criteria of the ticket (which you read in point #1, right). Don’t forget that tests are still code that needs to be clean and maintained in the long run, so its quality should match that of the “core” codebase.

Generally missing tests are a bad sign. Unless there is a very good reason – for example the PR is fixing a very critical bug on production and the fix was tested by half of the company – you should work with the author to get some in. Or at least identify the blockers why tests could not be written at this point and create tickets to address them in the near future.

5. Focus on logic, algorithms, context

It’s time to focus on the code itself. Start the review from the core files, following the logic to the less important ones. Think about missed business requirements, edge-cases, how these changes fit in the codebase and their context. You should understand what each part is doing and why. You should see how it achieves the task at hand (as laid-out in the ticket). If you see some big issues, for example it fixes the wrong issues OR the design is completely wrong, flag this to the author as soon as possible.

6. Complexity is the enemy

This point is so important it deserves its own section. Accidental complexity is the nemesis of a clean codebase!

After you are convinced the code is correct, you should focus on reducing complexity as much as possible. Clean code reads like a story and “speaks” by itself. Can the same task be achieved using better class composition, different design approach or less code? If there are “easy wins”, point them out and ask the author to adopt them. If major changes can significantly improve the code, this usually uncovers a process flaw – perhaps the technical approach was not agreed before starting the work. Note the opportunity for process improvement and if changing the code now is not an option – create a ticket for it in the future.

While at this point, always keep the KISS and YAGNI principles in mind.

7. Mentor, don’t dictate

Code reviews are one of the best knowledge-transfer tools you have in your team, so use it wisely. Always be friendly and polite. Show ways to improve, don’t blame for mistakes. Propose solutions, don’t just complain about problems. Going the extra mile and being truly helpful in code-reviews nurtures great team culture and positions you as a leader.

Recognise the great work of your colleagues. Code reviews are not only about finding ways to improve, but also a wonderful podium to thank them for their efforts. When someone does a particularly great job – draw the attention of the rest of the team to it and recognise it in the code review.

8. Stay pragmatic

At times there will be heated discussions and disputes that start in a code review. Quickly move them to in-person communication, where they can be resolved faster and in a calmer attitude. Keep the arguments driven by data and principles, not by ego and preferences. Stay pragmatic and flexible. There will be times when your suggestions are gratefully accepted. Other times they will be rejected, even though you are convinced you are right. Be willing to compromise, always remembering the shared goal of your team. In a highly “gelled” team, serious arguments will come rarely. Take advantage of them – see them as breaking the daily routine and as a sign you are challenging the status-quo.

Crafting and reading beautiful pull requests is a skill that needs time and lots of practice to master. Another great resources on the topic are Google’s code review practices, this Stackoverflow blog post and this data-driven article. Remember that the code-review practice will become easier in time. Once a team is fully “gelled”, all rules set here and in the previous post will come naturally and would not require any oversight. Until then – foster a positive team culture and soon you will reap the benefits.

Happy code-reviewing!