TLDR How to Do Code Reviews
I got lucky to find a series of great articles by Michael Lynch named «How to Do Code Reviews Like a Human». The author is sharing his experience of the topic and highlight many important psychological points, paying attention to which, you can dramatically improve development process in a team.
Part One and Part Two is available for you to spend some reading time.
Here is TL;DR version of mine for you, containing important highlights from the articles with my comments. I encourage you to get through highlights first and if you like content feel free to get through the comments.
TL;DR
Fill PR description and maintain changelist
«Before the code review begins, the author must create a changelist. This is a set of changes to source code that the author wants to merge into the team’s codebase.»
I’ve come across a lot of PRs which has failed this quite trivial requirement. IMO, it is hard and meaningless to make a review without a brief understanding of the thoughts and design behind an implementation and intentions.
Be polite and friendly
It is extremely important to keep in mind that:
«It’s easy for an author to interpret criticism of their code as an implication that they are an incompetent programmer. Code reviews are an opportunity to share knowledge and make informed engineering decisions. But that can’t happen if the author perceives the discussion as a personal attack.»
«The author can’t hear your tone of voice or see your body language, so it’s even more important to articulate your feedback carefully. To an author who’s feeling defensive, an innocuous note like, “You forgot to close the file handle,” can read as, “I can’t believe you forgot to close the file handle! You’re such an idiot.”»
«Never say „you“. Their natural reaction to hearing criticism of their work is to feel defensive and protective.»
So, be polite and friendly. Try to put yourself in an author shoes reading your comment and improve your wording until you got a thought: „It is a little bit over-polite.“ In that state, it will be polite enough for your reader.
And don’t interpret literally the «Never say „you“» section. Personally, I would rename it to «Avoid saying „you“.» Hope this little hint will help to understand the author’s point better.
Frame feedback as requests, not commands
«Code reviews require more tact and care than usual communication because there’s a high risk of derailing the discussion into a personal argument.»
«People like to feel in control of their own work. Making a request of the author gives them a sense of autonomy.»
One of the most important points from the article.
Lack of arguments and lack of opinion support makes review in-constructive, aggressive and inefficient. IMO, everyone should pay a lot of attention and put effort to support a review feedback with objective arguments.
Tie notes to principles, not opinions
«When you give the author a note, explain both your suggested change and the reason for the change.»
«Software development is both an art and science. You can’t always articulate exactly what is wrong with a piece of code in terms of established principles.»
«Provide supporting evidence where possible in the form of links.»
Basically, this is the continuation of the previous one. Using this approach we share values and knowledge between each other making our collaboration effective and positive. IMO, must have in every review process.
Aim to bring the code up a letter grade or two
«While your teammate might, in theory, want to explore every opportunity to improve their code, their patience is finite. They’ll quickly grow frustrated if you withhold approval round after round because you keep thinking of new and brilliant ways for them to polish their changelist.»
Truly. Empathy here is the key to success.
Respect the scope of the review
«If it’s egregiously bad, file a bug or submit your own fix, but don’t force it onto the author’s plate in this review.»
Totally vote for this one. In the team I’m working in, we agreed that little improvements could be committed to the reviewing branch with verbal author approval.
Limit feedback on repeated patterns
«When you notice that several of the author’s mistakes fit the same pattern, don’t flag every single instance.»
Let computers do the boring parts
«Automation helps you make more meaningful contributions as a reviewer. When you can ignore a whole class of issues, such as the ordering of imports or naming conventions for source filenames, it allows you to focus on more interesting things like functional errors or weaknesses in readability.»
«Plus, if they have to hear about a dumb mistake they made, it’s much easier on their ego if they hear it from a computer instead of from you.»
I’ve got and seen a lot of comments like „Variables assignment align“ or „No new empty line after the…“ I suggest instead of spending time searching and commenting on such a kind of cases, just invest this time into the automation.
Settle style arguments with a style guide
«Arguments about style are a waste of time in reviews.»
«Once you have a style guide, you don’t have to waste review cycles arguing with the author about whose naming conventions are best.»
Just make a Style Guide, and Slack channel or Confluence page for the Change Mangement process.
Start reviewing immediately
«Treat code reviews as a high priority. When you’re actually reading the code and giving feedback, take your time, but start your review immediately — ideally, within minutes.»
«If you’re forced to decline reviews more than about once per month, it likely means that your team needs to reduce its pace so that you can maintain sane development practices.»
Be generous with code examples
«In an ideal world, the code author would be thankful for every review they receive. It’s an opportunity for them to learn, and it protects them from mistakes. In reality, there are a number of external factors that could cause the author to perceive the review negatively and resent you for giving them notes.»
«A great way to make an author feel good about the review process is to find opportunities to give them gifts during the review. And what’s the gift all developers love to receive? Code examples, of course.»
Maybe it is written too much naive. Still, it is worth to keep in mind and use time from time.
Instead of summary
I hope those excerpts were helpful for you and they give you a hint how to improve review process inside your team.
Here is another excerpt from the article for a bottom line.
That’s one reason I don’t miss IT, because programmers are very unlikable people… In aviation, for example, people who greatly overestimate their level of skill are all dead.
— Philip Greenspun, co-founder of ArsDigita
Happy review, guys! ☺️
Hit the 👏🏻 and share if you find it useful.
Thanks for reading.
