Egoless Programming: Code Review

Table of Contents

What actually is a Code Review?
Reviewer perspective
Reviewed perspective
Further reading

There are dozens of articles about Code Review (you will find some links at the bottom of this article), but from time to time it is a good idea to revisit the basics or just find everything you know about the topic for industry newcomers.

I won't show you the tools [¹] nor try to convince you that you should make code reviews. In this article I will try to remind you of the basic rules and answer one simple question:

What actually is a Code Review?

Code Review, in simple terms, is a process where you manually review code written by other developers in your team. The goal of the process is to care about code quality, but also (if not most importantly) to improve knowledge sharing throughout the team.

It can be done commit by commit or on pull requests. Asynchronously or in pairs. The approach that you choose is less important; just agree on an approach that fits your team and project and start doing it. What is important is to make Code Review a regular habit.

During Code Review you are in one of two roles - the reviewer or the one being reviewed.

Reviewer perspective

1. Judge the code, not the coder

You have to remember that your main job is to check that the code is valid against your team's common rules and conventions [²]. You also have to check that the code doesn't contain any technical or logical mistakes. You should also use your experience and knowledge to share better solutions on how to write cleaner or simpler code that is easier to maintain.

2. Encourage, not discourage

In your comments, don't attack or criticize the person who wrote this particular piece of the code. You should always try to write in a friendly manner that turns the reviewed developer's focus on your technical comments. You must hold your tongue and not start any languishing discussions.

When possible, argue with examples and use references to the rules or articles that could help to explain the problem or solution. Try to omit forcing solutions with words like because I said so. It is equally important that the developer should discover how something should be done and why it should be done in such a way.

3. State questions and force thinking

Try to propose solutions, starting with questions like What do you think about... or How do you like.... Suggestions about different implementations should always direct substantive discussions, not proofs of who is smarter or forcing a different point of view.

Such questions should also force the developer to reflect instead of blindly copy-pasting a solution, so don't provide them with one. Just create a starting point.

And don't be afraid to use the word please.

4. Learn

Don't hesitate to ask questions if you don't understand something. For the reviewer, Code Review is a great opportunity to learn something through reading different solutions and arguments about why things have been built in such a way. Just remember that there are no stupid questions.

5. Care about team spirit

Be patient. Remember that everyone makes some common mistakes, so if you see them repeatedly, it doesn't mean that someone is stupid, only that you're probably ~~old~~ experienced.

If you like someone's solution - praise them. Coders, like everyone, feel good being scratched behind the ear. It's motivating for the job and it is always good to know that the thing which you spend a lot of time on really makes sense.

And last but not least, remember that from time to time you can be wrong and a different point of view doesn't mean they are wrong. If you are not able to defend your thesis with strong arguments, it probably means that every side of the discussion is right in some way and further comments will be just art for art's sake. You have to accept that sometimes things will be made in a different way that you personally prefer.

Reviewed perspective

1. Don't get offended

Remember that every comment, even nasty ones, are to the benefit of the project and don't reflect directly on you. A rejected commit is not a personal insult, but is part of the team work required to maintain a high quality product.

2. Remember that it is not your code

In most cases, projects are made by a team who can be internally shuffled. Code that is clear and obvious for you doesn't mean that it is the same for others. Every comment about readability or standards is not about personal preferences, but about care that the project could be maintained by every team member.

Reviewers who don't know anything about the project also might not know much about the business logic inside the project. This leaves reviewers with a frame of reference for understanding the code that may differ significantly from yours. If they make comments about code readability, it is a good sign that too much of the code depends on your knowledge about how and why something works.

3. Discuss and learn

Most comments bring some knowledge and learning. Try not to make the same mistakes again. If you don't understand or don't agree with comments, then always ask for argumentation. When possible, turn discussions into part of the project or company-wide documentation about standards. This is how our guidelines were born.

4. Make your and other lives easier

Small commits help to focus on the purpose of changes. As a bonus they have a positive influence on code review quality. Remember that a good commit message can often dispel reviewer doubts. Don't underestimate agreed upon rules and don't ignore them without a good reason.

Don't ignore reviewer comments. Take your time to respond and apply corrections if possible.