We all know it and hopefully we all do it: Code Reviews! But have you ever thought on how your comments may be perceived? Or have you ever been on been receiving end of a frustrating comment on your code?
When I talk about code reviews, I mostly aim for code reviews done on pull or merge requests in a written form. But most of this probably also applicable to actual face-2-face/pair code reviews.
Code reviews are something very intimate for developers, as we share our work with others, in order to get feedback. Sometimes, feedback can be harsh, and this is maybe why people are sometimes afraid of opening their code for to review.
But wait, shouldn't there be a common way of communicating feedback? Even on things that didn't go so well or could be improved? Maybe in a constructive manner? Oh yes, there is a way!
Here are a just few bad examples of rather less constructive comments on a pull request:
- This doesn't any make sense.
- I don't see the point of this.
- This SHOULD NOT be implemented like this!!!
- Why are you doing this?
- I don't like this.
This are just a few examples to highlight bad and not constructive comments. Depending on how those lines are being read and perceived, they are also destructing the confidence and working morale of the review seeking developer.
Furthermore, any of those comments are the perfect start of a long, long, long text-based argument. And instead of having a few constructive comments and improvements, you'll end up with an 80 comments long code review where everyone is defending their own view.
Is that helpful? Probably not!
Okay... But what if something is wrong with the code or you really don't understand what is going on? How about not writing comments, just approve and refactor it silently later? Mhm... This doesn't seem right either, ha?
Here are few tips on how to improve comments on code reviews:
Instead of just highlighting an issue, offer alternatives or ask for a quick white board or on-screen discussion.
The comment "Why?" is not really question. How about something like this: "I'm having trouble understanding this, maybe I'm missing something. Do you have moment to explain this to me?"
DON'T WRITE IN UPPERCASE, AS IT IS USUALLY BEING READ AS BEING A BIT AGGRESSIVE!!!
Instead of writing "You be should doing it this way: [...]", how about: "I've a few thoughts around this one. Could we meet for a quick chat about this?". Or maybe: "Nice! I recently found this on www.dev.to/some_article. Maybe you could give this one a try? Let me know how it went."
Writing an assay about something in code review is usually not a good idea. This usually leads to very unproductive text-based discussions and sometimes loses track of the actual issue. Create a ticket for the topic to talk about or set up a meeting. This way it will be more open for a wider discussion with the whole team.
Do you like to read bad and not constructive comments on your code? No. I thought so much. Talk to people the way you would like to be talked to.
Saying phrases like: "Good work", "Good step in the right direction" or "This a good improvement", don't hurt to write. And even if the code in review has some flaws, someone spend time to write it and had the guts to open it up for people to review. This should be celebrated with appreciation!
This list could probably be extended and improved on, but I guess the message is clear. I believe, if everyone respects and follows basic communication guidelines, it will lead much less friction within discussions and will enable a team to focus on the cool things, like: Making Awesome Software together!
Do you know any no-go-phrases you have seen in code reviews?
How do you deal with bad language within code reviews?
How do you deal with a change you don't agree with as a reviewer?