I love reviewing code. I love reading it, thinking through it, and really understanding what my fellow teammate is trying to accomplish with it.
Unfortunately, I’ve don’t always review code they way I would like to.
And while I wish that weren’t the case, the truth is that many software engineers struggle always to leave helpful and constructive code reviews. And that’s OK — we are human, after all. We might be having a bad day when you get that PR with 100 files changed, or you might have been harping on a pattern someone decides to not follow in the next. In either case, it’s OK to be an imperfect reviewer.
However, there are a few things you can keep in mind that will help you from writing that comment that creates a hurt relationship on your team. Here are a few things I have found that never go well when doing a code review that you might want to avoid:
Asking Condescending Questions
Asking questions is great! But have you ever asked a question that was sharper than a statement? The questions where you’ve already made you mind on something, but decide its better for them to figure it out on their own. The times you are asking a question just to drive home a point?
Examples might include
- “Umm, how long did you spend working on this?” — when code looks like it was written a little hastily and a few details are missing
- “How long did you say you have been using this framework?” — when it seems like a developer didn’t understand how to use a specific framework correctly
- “What are you doing?” — when something is blatantly wrong
- “You know this doesn’t work like that right?” — when a developer didn’t understand a library correctly
Questions like this can easily make someone feel attacked. Even if they have made a mistake (which we all do), chances are they won’t want to be willing to admit it or compromise after feeling like that. When we feel attacked, we start to get defensive, our fight or flight techniques kick in, and our ability to think clearly shuts down. Literally.
But you can ask all of these questions in a much friendlier tone:
- “There look to be a few pieces missing to me. Did you just miss them or where they left out intentionally?”
- “I haven’t seen this done way in this framework before. Can you explain how it works or what you are trying to accomplish?”
- “I don’t think this looks right, but I could be wrong. Could you explain what this is supposed to be doing?”
The key here is extending an invitation for the author to explain their intentions and goals rather than you determining them for yourself.
Mixing Opinion and Facts
As developers and engineers, we all have opinions on code. That is all well and good. When things go wrong, though, is when we mix our opinions with facts about code. We become obsessive over things like including readability, naming, or the best syntax when using a for loop.
Why does that happen? While I can’t speak for every developer, for me, it had a lot to do with the code I saw from the senior engineers on my team. If they named their variables long, I wanted to too. If they preferred yaml to json, I tended to as well.
Regardless, these sorts of issues find themselves wasting time and energy. Agree on a code style, pick a linter, and move one.
Of course, subjective opinions and issues will come up. When they do, always come at it with a humble tone. “Consider naming this accounts instead of accountList" or "maybe we should consider a refactor here since this class has gotten rather large."
For things that are facts, you should do your best to provide references. And not just references to other opinionated articles! You can provide those when you discuss and suggest changes based on opinion, but not for facts. When stating a fact, reference RFCs, library APIs, official documentation, etc.
Forgetting About Praise
When most people talk to me about code reviews, there is a focus on how to share criticism effectively. How to share a thought (fact or opinion) about how to improve the code you are reviewing. They want to help their teammate learn and grow, so they want to point out the mistakes being made and the areas for improvement.
That is all well and good, but I know when I submit code for a review and all of the feedback is negative, that doesn’t motivate me to become better. It just makes me feel terrible.
Even if it’s a small thing, you should find SOMETHING to praise in the code you are reviewing. I doubt its 100% bad. Even if its someone’s first pull request, you should praise them for the courage to open one!
Forgetting You Have Something to Learn
As a reviewer, you might think that you have the ability to pull a Gandalf:
But that shouldn’t be your goal. Your goal should be to learn! You should be trying to gain understanding! Learn the reason behind why these changes are being made, learn how these changes accomplish that goal, learn new patterns or tricks you didn’t know before.
When you fail to remember you have something to learn, you will simply miss out. You will miss out on your own chance to learn valuable tools. You will miss out on the opportunity to build a better relationship with your teammate.
If you can, stay away from these behaviors when reviewing code. Build trust within your team by having humility, asking good questions, and having a learning mindset. Separate your opinions from facts and provide the right references for both. Remember to encourage your teammates with the good code they have written — emojis are great! 😁
Happy coding!
Originally posted on Medium
Top comments (2)
This was the advice I needed today, thanks for such a thoughtful blog.
I'm glad you found it helpful! Thanks for reading!