Translation: Instructions for conducting code review

Not so long ago, my colleague translated an interesting article on code review, I liked the translation. And this morning, the tangled count of crosslinks brought the eyeofhell to an even steeper article. Your attention is invited to the translation of a brief but concise instructions on how to make a review of someone else's code and survive the review of their own. Unlike the above-mentioned article, this one focuses more on the practical aspects of code review and contains many useful recommendations on how and what to do so as not to be painfully painful. Hint: to read the original, click on the author's name in the plate under the translation.

For all participants code review

')

• Take for granted that many architectural decisions are a matter of developer’s personal taste. Try to discuss only the strengths / weaknesses and decide as soon as possible what to do with the code.
• Do not ask, but ask questions. For example: "What do you think about the use of the identifier name: user_id?". Translator's note: this advice is based on the observation that the human brain tends to give a strong emotional response to criticism and demands, while the invitation to the discussion often passes by the emotional component and allows you to calmly communicate about the code.
• Do not hesitate to clarify incomprehensible moments. For example: “I can not understand what is happening here. Can you explain in more detail? ”
• Try to avoid dividing the code into your / someone else's. Be suspicious of the words “mine”, “not mine”, “yours”.
• Discuss the program, not the programmer. Try to avoid becoming personal and do not use expressions like “dumb decision” or “idiotic code”. Perform code review as all team members will be - smart, intelligent people with good intentions :)
• Try to express your thoughts explicitly. People are not always ways to understand your intentions online.
• Try to be more modest, for example: “I'm not sure that this will work, let's check?”.
• Do not exaggerate and, if possible, avoid such words as “always”, “never”, “nothing” and so on.
• Sarcasm is also not a good thing.
• Try to be yourself, no matter how jaded this phrase sounds. If emoji, animated gifs and humor are “not yours” - then you should not force yourself, trying to be like other team members.
• If there are too many “I did not understand” notes in the discussion and “there is another way to solve this problem,” then it would be a good idea to discuss this code personally, and then write a short follow up as part of code review.

If your code has undergone code review

• It is considered a good tone to thank the reviewer for the work done, for example: “Thank you for finding this interesting situation, we will change the logic of the completion of the work”.
• Try not to take the code review results to heart. Discuss the code, not you.
• In controversial situations, try to explain the purpose for which this code was written. The answer to the question “why?” Simplifies communication.
• A good idea would be to make voluminous corrections in separate tasks, and not to put everything in one place.
• Link to code review from the relevant task, for example: “done, you can review: github.com/organization/project/pull/1 ”.
• Do not combine commits (squash) until the review is complete. Reviewers should have the opportunity to look at individual commits that fix specific issues on the code.
• Try to understand the point of view of the reviewer.
• It is considered a good tone to respond to every comment.
• You do not need to do merge until all the continuous integration integration tests have been completed.
• Merge is recommended to be done only if you are confident in the code and its impact on the project.

If you want to subject code review to someone else's code

First of all, try to figure out why this code was written. Is this a bugfix? New feature? Refactoring? Bloody mess of all of the above? Then:

• Try to explicitly indicate in which remarks you are completely confident, and in which not so much.
• Guess ways to simplify code without losing functionality.
• If the discussion of the code goes to the philosophical or academic jungle, then such a discussion is well taken offline. For example, on the traditional Friday gatherings. Is there a Friday gathering in your team?
• Offer alternative solutions, but proceed from the assumption that the author has already tried them. For example: “what do you think about the custom validator?”
• Try to understand the point of view of the author of the code.
• At the end of the code review, mark the pull request with the comment “you can merjit”.

Code Design Notes

If you see that the code explicitly violates the coding style adopted by the team, it is useful to point this out during code review. If it turns out that someone on the team does not agree with this style, then the discussion is recommended to be put on a separate ticket.