Ethics of Code Review

One of the skills that every software engineer must acquire through his/her journey in development is how to review other people's code, this process is called Code Review.

Through the ages of writing code, the code review process has evolved over time to be in an automated manner. Nowadays we have automated code review systems e.g. Danger which can automate some of the code review steps that are required for quality of code, for example checking for file changes count, linting the changes and other required checkpoints.

When you come to play and asked for a code review from a colleague, sometimes you got confused about how to deliver your code review the best way, if this's the case, this post is for you.

So, What are the ethics of Code Review?

1- Represent yourself

When you review some colleague code, it's hard to decide how to address some confusing code block, however, the good news is it's only you are reviewing. It might be not confusing for others, or the world can understand it, but you need some clarification. That will be a very good reason to use the "I" statement to indicate it's your opinion not obligated to anyone else.

Good Examples:

I can't understand this code block, could you please explain what's the purpose behind?

I am confused about the reason this line of code is placed here?

Instead of:

This code block is not understandable

This code block is so confusing

This way you just refer to yourself, not necessarily all other reviewers share the same opinion.

2- Be Rationale

So, you read some code, find out this code introduces a new wrong behavior, or miss something that must be addressed. You just added some comment like

This Code isn't connected to backend and will fail eventually

. From the code author perspective, it might be confusing why this code needs to be connected to backend and why it would fail.

Adding some rationale besides this comment will make it shine like a fact. So prepare for the reasons why do you think of this comment.

Good Examples:

This Code isn't connected to backend and will fail eventually, as the backend service start function not called

Instead of:

This Code isn't connected to backend and will fail eventually

3- Question not Demand

The best part of any software solution is the outcome and impact it provides for the problem it's designed to solve. Code reviews are a solution designed to improve code quality and sharing the responsibility of code modifications across different members, for this, as any improvement process, it's always better to boost the discussion culture between team members, asking questions is the key to start up a conversation between the reviewer and author. It's not only important for the current code under review, but it's also for any futuristic modifications.

Good Examples:

Could you check the issue of closing the connection after successfully read data from the server?

Instead of:

You've not closed the connection after reading from the server

4- Propose a value

The original purpose for code reviews is to improve, either as a developer or code quality, hence a better working code for all of the contributors.

Thinking about the value you have to consider if, for example, the context is pushing a hotfix, focusing on spelling won't be the best value you add here, instead suggesting a spell checker step futuristically will make much more value.

Good Examples:

What about adding a spell checker to be integrated into the IDE <IDE name > to check to misspell?

Can we add a code formatter step?

Instead of:

Adding multiple time:

Please fix typos

or

Fix the misspelling in all code changes

5- Don't make fun of code

Usually, written communications strap out the emotions and feelings of the words, writing sarcastic words might be sensed from code author, or might not understandable at all. But in all cases, it's not a positive value. So leaving sarcastic words out code review will eject any negative vibes from the delivered feedback.

Good Examples:

I believe this code block can be written in a more functional way, what do you think ?

Instead of:

This code block miss the professional hands

6- Be objective

When writing a feedback about some code changes, it's always positive to keep it about the code itself, not personally addressing the code author or blaming him. Putting the mode of personalization is always considered the worst solution for giving any feedback especially in written communications.

When you really see issues in some code changes, address it by code not by the person who writes it, this way you're reviewing a piece of code whoever wrote it not relying on some expertise of its owner.

Good Examples:

Could you check the issue of closing the connection after successfully read data from the server?

This code is missing the failed connection error handling, could you check please ?

Instead of:

You've not closed the connection after reading from the server

You should handle the failed connection error

7- Avoid judgemental words

Words are like good or bad indicators of respect between members over a project. On a code review, body language, facial expressions, and emotions aren't present in written communications, so avoiding phrases like "It's just easy", "It takes no time", "It's a trivial task" and "It's obvious in code" and trying to re-phrase them in a more compassionate way will build a mutual learning relationship between code author and reviewer.

Good Examples:

I believe function ABC can provide same functionality could you check it please?

What about developing a new approach that utilizes remote connection?

Instead of:

Function XYZ is obviously the same as ABC

Please develop a new approach for remote connection, it's just easy to break this one

8- Be aware of the author context

When reviewing some code changes, you have to consider the code author's context, is he/she a new hire? adding a hotfix quickly? from another team? or pushing some architectural changes?.

Understanding the context of the code changes will determine a couple of things; First, how much reasoning and guidance do you need to post as it will vary from one context to another. For example, if he's from another team, he might not be fully aware of the linting strategy the team is following so might leave some rules unchecked.

Second, what category of code review is expected, for example: pushing some hotfix shouldn't fall in deep code review where a warning must be fixed before release, instead focus on the approach of fishing broken functionality.

Good Examples:

It seems this function missed linting, Could you please check our team coding guidelines?

[Hotfix]: In order not to have this issue again, what about adding a flag for function XYZ to enable/disable using it?

Instead of:

Linting functions is a must in our team philosophy

[Hotfix]: This fix is missing proper naming for branches

9- Don't play the instructor role!

Sometimes when you review some code, you could see some instructions to be directed to code author something like: "Don't do this, it's an awful way". At this point of time, you need some good amount of wisdom not to trap yourself playing the teacher role, this for a couple of reasons; First, you don't review code because you want to instruct or technically teaching code author some programming skills, your responsibility not to allow an unaligned piece of code to enter codebase. Second, playing a teacher rule will put the conversation into a master-slave mode which considered toxic for the work environment as a whole, the relation between code reviewer and code author should put aside any titles or positions and only focus on code under review.

Instead, put a conversation on learn and discovery mode which both code author and reviewer (you) explore problem domain which code changes to address and learn new ways of solving them.

Good Examples:

It might be a good idea to segregate this interface into multiple ones to avoid fat interfaces syndrome

Instead of:

Segregate this interface to multiple ones, we don't want fat interfaces to leak to our codebase

10- Expect misunderstandings

Despite the high-level language ability to talk to machines using human language, in the end, all our code talk to machines. This fact encounter that human behavior to translate commands to the machine might be misunderstood at the writing stage, you might write some code to another is not understandable at all even you both talk the same language.

When reviewing other developers' code from another team or even from your team members, expect the miscommunication that might arise due to disparity of seniority, experience, the scope of work, or personal preferences.

In this case, avoiding harsh or critic words and asking for re-phrasing or showing the rationale of code change will help both sides build a trustworthy code review that propagates the code review culture.

Good Examples:

What about re-phrasing this function to be simpler and more readable ?

Instead of:

I can't understand what this code does... totally confused!

Conclusion

Code reviews are opportunistic occasions that leverage the ownership of codebase to multiple players. Imagine thinking with different minds and versatile mechanisms for solving a specific problem with code. It's when you review some code, you lend your thoughts to another colleague with the intention to make codebase better, following the above-mentioned highlights make this process fruitful and evolving for work environments.