Do Code Review As If Your Life Depends On It (Because it Does)

You are going to do a code review, great. Here is a guideline to make sure you are going to do the best code review in your life.

For the last 5 years, I was mostly doing code reviews for smaller and larger teams. Basing on my experience, I prepared a guideline for anyone who wants to be better at doing them. This blog post is also used internally in Ulam Labs as a first thing to read before doing code reviews and is subject to change in case of internal/external feedback.

WARNING. This article is targeted to mostly frontend/backend developers, if you do embedded work or coding in other areas, some parts could be not related to you.

How to do good code review?

Mindset

Code review is based on trust

You do a more detailed code review for a new developer or junior than for your 2-year fellow that you have done 1,000 code reviews and trust deeply. Dead simple.

Understand the code as much as the developer who wrote it

Going line by line is not enough, you need to take a deep dive into the code. This is because CR is not only about code quality but also about making sure there is more than one developer who understands the given piece of the code.

The other thing is that without deeply understanding the code you are not going to do the best code review. Not even a good one.

Good code is not about formatting

It is, but formatting and code quality should come after review. There is nothing more frustrating than poor code review that just points out formatting issues (or other silly things) while the actual errors are not found by the reviewer.

First, think about business requirements before code review

This is the most important part and the best time to answer these questions:

1. does the code cover all business requirements described in the story or bug?
2. do the tests cover all business requirements?

If you do that part properly, you will avoid situations where the developer forgets about one tiny business requirement at the end of the story description, resulting in a bug in the next sprint ( there’s a chance QA would catch that first, you shouldn’t rely on it).

Second, think about edge cases

This is a crucial part and the hardest one. First, you have to understand the code as much as the developer who wrote it. Then you need to think deeply about what could go wrong during that feature usage. Next, check if the solution is robust enough to handle current system load (for example offloading intensive things to background task). Check if all error scenarios are properly handled, like input data validated or all possible exceptions handled (and tested), like errors from 3rd party APIs.

Third, think about tests

It is good practice (and I often do it), to start the code review from tests because they should show how the feature works. Tests have to be easy to read so that you can extract all business requirements from them.

If there are missing tests I am pointing that out first. After reading and understanding the code, you come back to tests and check that all edge cases are also covered.

Code review execution

Ask the author to code review himself

Before doing code review, I often ask the author "please do code review yourself first".

Here, the author should take 10,000 feet view at the code and check the following points:

• if the code covers all cases mentioned in bug OR all acceptance criteria in the story? Open tickets and double-check them!
• if tests cover all cases mentioned in the bug or all acceptance criteria in the story?
• if code is not too complicated or it made existing codebase too complicated, maybe this is a good moment for doing an old codebase refactor? if so DO IT --> Read our blogpost about Why refactoring your code is important?
• does the code look beautiful, are you proud of it? take a coffee break and read the code in the PR, make sure you love it.
• if commits are properly squashed, commented and code properly segregated, in most cases you need 1 commit = 1 bug/story.

Pull the code, open IDE, run the tests

Often you have to see the code in the IDE. Why?

• To be able to run tests yourself.
• To be able to find usage of some methods (you won't be able to spot unused method in the PR, especially if it is not in PR itself)
• To be able to modify the code and rerun tests (to check some possible simplifications or enhancements)
• To be able to push a commit with missed tests (or other code improvements). Right, the perfect way of doing CR is to push tests that fail but should not :)

Make sure you are understood

When you leave a comment, make sure the author will understand you. Your comments should be precise, with proposed solutions and code samples, so that the author does not need to come back to you to ask for an explanation.

Especially these kinds of comments should be avoided:

• "I don't think it will work" or "Are you sure that ...". Give proper scenario AND what will be the (unexpected) result. You are not sure? Pull the code and find out.
• "Looks good but please improve test coverage". Code coverage is not everything, showing that you care about coverage will result in stupid tests that aim to gain coverage. You should point out what cases are missing in tests.

Providing a line of complex code without any explanation will only bring more questions and unnecessary further discussion to the PR - always try to explain “why” along with such suggestions.

Do a code review in one run

There is nothing more annoying than a code reviewer who forgets to point out things and mentions them in the second run after CR fixes have been applied.

This is lame. This way you are losing respect for your colleagues, make sure you don't do that.

Push your proposed changes

It is an excellent way of doing code review by pushing a commit with the proposed enhancement or missed test cases. Sometimes if the feature overwhelms the author, you can push a commit with a completely different, not polished (but proper) solution to put the author on track.

Don’t postpone the code review

Code review is the most common factor that blocks features from being delivered. I know that it is exhausting and hard but it is the key  when delivering quality software.

Let me give you a typical example of how the life of PR looks like:

1. The author opens PR, thinks that his work is done and starts working on another feature.
2. You are procrastinating the pain of coding CR but after 2 days you do it.
3. The author is also procrastinating, so he prefers to finish another PR before switching contexts to the old PR. Another 2 days lost.
4. Author applies the fixies and the PR is ready (of course only if you did the best code review)

So we have like 4-5 days from PR being open to PR being merged, assuming you did your part well.

How can we do it better? Start the day from code reviews!

This way, assuming authors are also applying code review changes in the morning, the above scenario shortens to 2 days. This is a big deal and would make a great impact on overall team performance.

Remarks on code review

Code review is exhausting

One thing you should understand is that code review can be very exhausting and quite time-consuming. Someone was developing the feature for almost 2 weeks? And now you have to understand it in minutes or hours, you will be brain dead after that.

What helps here is pair programming, it is much easier to understand the code if the author himself, next to you, can explain to you the details.

It is a must-have if you are not feeling senior yet and it is good to do it if a change is substantial.

Don't ask stupid questions in the PR

The second most annoying thing is asking questions that are easy to find in the PR or code base. This bloats the PR with unnecessary comments.

If you don't understand a piece of code:

• pull the code and open in IDE, run tests, debug it
• do pair programming, ask your colleague to explain things personally

Don't be afraid to admit that you don't understand the code

You could have a bad day, coffee was not strong enough or the feature is really complex. Don't be afraid to do pair programming, ask the author to explain the hardest parts. Only if you fully understand the code you can do a good code review.

Treat this as an opportunity to pass on knowledge

A great code review should also be a source of good practices and should show the reasons for requested changes so that the author of the code can learn from it and prevent such errors in the future. This is especially crucial for junior-level programmers, for whom such feedback can be valuable.

Be respectful

Don’t forget that on the other side of PR, there is a human being. Be humble, don’t think your time is more important than his. Make sure you are polite and people eagerly wait to learn from your code review!

‍