Get best of the week

Code review Terminator. Review for which you will be thanked


Ginger helps me review the code. And when he doesn’t like something - also a real Terminator,

“Code review Terminator,” a colleague once called me after a particularly productive review. On the one hand, it entertained the Czech Republic and was pleasant. On the other hand, a colleague really learned something new, and this allowed him to write better code. So win-win.

After the change of work, colleagues also changed. But in a new place, too, they began to thank for the review. I decided to figure out why, and put it on the shelves. It turned out 11 recommendations.

1. Treat the review as one of the main activities


Leaving a couple of comments like “And then there isn’t enough null checking” is not enough. It is necessary:

  • Figure out what needed to be done
  • Understand how it was done
  • . ( , , )?
  • , , . — , .

2.


It's one thing to sit next to a colleague and discuss something, looking at one screen. Another thing is to review the code on the github or bitbucket. It is easy to misunderstand the intentions of a colleague when communication occurs in text. Take the phrase “There is a bug in this line”. Yes, it’s clear that the phrase says that the article has a bug. But she could be told with different emotions: anger, surprise, joy that the bug was caught and did not get into production. Or maybe with indifference.

Your colleague may misunderstand your intentions - and this leads to resentment, tension in the team and generally sucks.

Make your life easier: soften the phrase, transform it into a question, or maybe add a smiley face.



3. Allocate time


To make sure the code works correctly and well, you need to fully understand it. And it takes time; make sure to highlight it enough. Remember that reviewing parts of the application that you don’t know will take even more time.

In general, this is the flip side of good reviews: it is time-consuming. If you don’t have time to do it well, try to postpone or appoint someone else (sounds like a procrastinator's advice, but situations are different). If this is not an option, but you need to do a review - remember that you sacrifice quality. Although necessary, but a compromise. If you turn it into a habit, you will get more technical debt in the future.

4. Do not make assumptions; ask


When you come across a weirdness in the code or an overly complicated solution to a seemingly simple task - do not assume by default that a colleague made a mistake or a bad choice. Perhaps he is aware of some circumstances that you do not know about. You can simply clarify and avoid the risk of uncomfortable and stressful situations if you unfairly blame someone. For example: “Maybe you can use such a class here? As far as I understand, he would be a good fit here and that would be easier than the proposed implementation. ”



5. Catch situations when you need to contact directly


Typically, reviews are performed asynchronously. So you can immerse yourself in the code and distract your colleague less. But in some situations, it is better to contact directly (by going up or calling):

  • When the deadlines are running out. Fast feedback speeds decisions. But neatly: with deadlines and so on, everything is cocked, and distractions will annoy and bring down concentration.
  • Gross mistakes. Discussing them publicly can be very embarrassing and frustrating for someone. It’s better to talk in person and explain the problem. Maybe it's just an oversight, or maybe a knowledge gap - which is now filled.
  • Completely wrong approach selected. To tell a person that you need to throw away his work, you need to be careful. It is better to use body language and intonation to convey clarifications without offense.

Well, it’s a good idea to add a conversation summary to the review system.

6. Read the ticket first


Some of the requirements may not be covered in a seemingly correct pull request. To avoid this, just read the problem statement first. At the same time, this will help in understanding what is generally happening in the changes.



7. Justify your suggestions


Some errors (typos, for example) do not need explanation. But if you offer an alternative architectural solution or another name - explain the advantages and disadvantages of both options. What seems obvious to you may not be completely obvious to other people.

In addition, there is a proverb: “Give a man a fish, and you feed him a day. Teach him to fish, and you feed him for life. ” When you taught a colleague a new approach, he will be able to use it in the future and write code better. At the same time, the quality of the project will also improve as a bonus.

8. Praise good decisions


A review does not have to be a listing of errors. If you saw a good place, an interesting solution, a useful method - tell me about it. A brief comment “Cool decision” can improve a person’s mood for the whole day. Yes, and do not praise the bad pull requests: it is tense and destroys the meaning of the idea.



9. Be polite


Hint: a phrase like “ Can we please get rid of the brain-damaged stupid networking comment syntax style? ” Is not polite (sorry for the English here, but this is a direct speech by Linus Torvalds and it would be dumb to translate; he insists colorful not to use it a certain kind of comments, for the sake of courtesy, adding “please”).



10. Help


If during the review process you analyzed several files and as a result found an alternative solution - discard the colleague's links to these files. You can even with line numbers, it’s even more convenient. It will not take much of your efforts, but a colleague can save a lot of time.

11. Suggest, do not indicate


When you propose a different approach to the review - do not tell your colleague, by order, to use your option. Argument and let your colleague decide. Most likely, he will take your advice. And if not, what could be the reason?

  • Both approaches are about the same. If there are no objective reasons for choosing a new approach, then there is no reason to waste time and apply it. Disclaimer: code uniformity is an objective reason.
  • There is an objective reason why your approach is better. But for some reason, the author of the code does not understand it. Review your argument, explain again - and at the same time check whether you yourself are mistaken.
  • Personal conflicts. This is slippery ice and you need to act here carefully. And this is already beyond the scope of the review topic.


That's all. Summarizing:

Make the world around you a little better. Do good reviews.


UPD. This article is a free translation of my original in English . Converted from "translation" to "article" so as not to confuse readers.

More articles:


All Articles