Code Review Etiquette

the customary code of polite behaviour in society or among members of a particular profession or group
synonyms: protocol, manners, accepted behaviour, rules of conduct, decorum, good form

Etiquette is important for several reasons, one of them being that if you don’t abide by it, no one else will enjoy being around you.

With code review, not abiding to etiquette means that people will not enjoy reviewing your code, sending code your way for reviewal, or both.

There’s actually two types of etiquette when it comes to code review: the one from the coder and the one from the reviewer.

The person writing the code should have in mind that it will be reviewed by a human being and as such should try and make that person’s job easier.

The reviewer, on the other hand, should have in mind that the person who wrote the code is a human being and, as all human beings, has flaws and feelings, and those should be taken into consideration when commenting code during reviews.

That being said, here are some guidelines.

Etiquette rules for writing code:

Code Clean and Self-Review — someone is going to read what you write, so the least you can do is not make their job any harder; do read your own code before submitting it for review; make sure it does what it’s supposed to do and try to run some static analysis on it to clean up any possible problems.

Explain yourself — most comments are either meaningless or a sign that the code could be improved, but do leave a comment explaining your algorithm or your reasoning if it took you some thought to get to your conclusions; also, fill in the description for the Pull Request, or leave a comment there if you feel that something else deserves an explanation.

You are not your code — don’t take the comments from the reviewer personally; if someone attacks the code, don’t put yourself against them; put yourself at their side and comment the code with them; agree of disagree, but treat the code as an object, and not as an extension of yourself.

Etiquette rules for reviewing code:

Comment the code, not the coder — the focus of the code review is the code, and not its creator (if the focus happens to be its creator, then that’s not called code review anymore and you have a different problem on your hands), so watch the way you address any issue; try to refrain from expressions such as “you did this again” or “what you’re doing is wrong”; replace them with “we’ve had this problem before” or “this is not the best way to do this”.

Explain, don’t complain — granted, if the same person makes the same mistake over and over again, this might be difficult to manage, but the truth is that it is much more effective to say “there is a better way to do this” than “this is really wrong”, even (and especially) if you really believe that it is truly wrong; while you will get your message across with the second expression, it’ll also prompt the person who wrote the code to defend it.

Don’t let things escalate — we all know that things tend to escalate too quickly; if you do find yourself exchanging too many comments and find your limits being tested, do try to have a conversation in person instead of letting those comments escalate; if you see the need for this, do it as soon as possible.

While the lists above are not extensive, they do cater to some of the most horrific problems I have seen with code reviews that have actually led to teams degrading and code review being seen as something bad. Following the above rules should prevent you from going through those same problems, and you will also live a happier life.

Actually, there’s one missing from those problems I have witnessed:

Whatever you do, regardless of how much you hate the code you’re reviewing, or how many times the coder has made the same mistake, please, please, please do refrain from throwing stress balls at their head instead of talking about it.