One of my goals at work has been to review code along with writing code. I have been trying to review more and more code and though it's fun, it does come with its own set of challenges. I wanna talk about how I've approached it and what has helped me along the way as I try to get better at it.
It's not rocket science. Seriously, it's not. For a long time, I constantly thought of people who review code as people who have all the context to review the piece of code they were looking at, so of course, they'd find it easy and effortless to do that. This is rarely true, even for smaller companies, with systems that have lots of moving parts. Nobody knows everything about every part of the system. It's good to remember that. Reviewing code is a skill that can be learnt just like writing code. The more you do it, the better you get at it. Reviewing code also helps you write better code because you know what kind of things you need to watch out for. Don't let the fear of not knowing everything about the code you're reviewing hold you back.
-
Mistakes will be made. Something or the other will always slip through the cracks because humans write code and humans review code and we're all inherently prone to all sorts of errors. That is okay. Just because you can't do it perfectly doesn't mean you shouldn't do it at all. It's also completely okay to ask someone to take a second look if you're not sure about something. Having a second pair of eyes never hurts.
Tangentially related to this point is having a blameless culture where people don't hunt you down if you mess something up in production. The primary focus is to diagnose and fix the problem rather than assigning blame (who wrote/reviewed the faulty code). There is room to make mistakes and what's important is learning from them. I'm lucky to work somewhere that has this culture, so it doesn't feel as daunting. ๐
-
Code reviews are very ripe for misunderstanding and lack of empathy on either side. At the heart of code reviews is collaboration. It is as important to remind yourself as a reviewer that you're reviewing someone's code and not passing judgments on them as a person and it is equally important to remember that whatever your reviewer tells you is not meant as a personal attack. I feel a lot of this is dictated by the engineering culture of your team and the company overall. Being mindful of this before posting comments can greatly reduce any possibility of friction.
A very simple way to do this:
-
Another point I want to touch on is the junior-senior dynamic that comes into play when reviewing code. As someone early in their career, it can be very daunting to review code for someone who has a lot more experience and/or context than you. However, this shouldn't discourage you from reviewing their code. Some ways you can do this:
- Shadowing reviews on existing pull requests: this is a great way to learn to spot what kind of things folks watch out for when reviewing code and also how they communicate it and how it is received by the author (how you communicate feedback on a change is equally important, if not more than what you communicate)
- Pairing with another engineer or the author when reviewing something more complex: this let's you ask questions in real time and gain more context about what's going on
-
Asking for more context is okay. This can be scary at first when you're brand new to a team and/or domain and everyone around you seems to know so much more than you. If something doesn't make sense even after looking the relevant bits and doing your research, it's okay to ask for more clarification.
Let me give you an example of this:
Last week, I was asked to review a small PR for a service I have never touched or used before and my first instinct was to ask someone on my team with more context to do it instead. However, I took it as a learning opportunity and tried to figure out what was happening in the proposed change. After looking at it, even though the overall pull request made sense, some things didn't, so I gathered the courage to state that I'm new to this service and ask the author for more clarification. And guess what - they were more than happy to provide that! Once I had that, I was able to approve their change and at the same time, walked away with several things:- I'll be more confident about asking for clarification after this positive experience
- I've a better understanding of what that service does and can review related changes more easily
- We identified how we can improve the documentation for the functions we were looking at so that other folks can understand it more easily
It was a win-win in every way! ๐
Once I started looking at code reviews as just another skill that I can learn and get better at over time, I felt much more confident about doing them. It's less scary now and I look forward to doing more of them. ๐ช๐ผ
Top comments (6)
Code reviews are definitely a good way to learn because you can be exposed to different coding styles and designs. What helps me is having the team be consistent in what they are reviewing that way if you make a comment about something (ex. coding style: tabs or spaces) that is not normally not reviewed the author is not thrown off or offended. Excellent post.
Great post Shubheksha! I'm currently experiencing this at work. I'm reviewing code from merge requests where I know very little context about the project and I get to learn a lot with this (e.g.: about the technologies being used or getting a grasp of some projects within the company I don't get to touch very often).
Thanks for the great article!
One thing made me think:
Yes, of course. It should always be okay, to ask for context.
OTOH if someone reviews my piece of code and asks me for clarification or context, I take it as a sign to improve my code.
Usually I strive to make my code as understandable as possible - although there are times, when I have to redo passages. But I find it very important that my code is understandable relatively independend of skill level and context.
At times, I wrote code which I had reviewed immediately by newcomers on the team. And when they did not understand right away what was going on, I redid the passage until they understood. The downside: that takes time, which one -unfortunately - does not always have. But I wish I could do it more often.
Code reviews are extremely important. I do them on myself few times a week before my commits get to far ahead of my last review.
Great post!
Avoid statements in code reviews, such as "Avoid statements". That is an option. Maybe use "could this be X" and most importantly, ask questions. Be the beacon on code quality and make yourself essential.
One addition I would make is that if you are going to provide a correction, justify why and demonstrate the benefits of your solution.