Frontend focused full stack enthusiast.
30 January 2020
Code review can be tough. I know when I was first getting started, I tended to go a bit overboard in my code review. Even now, code review can take me a while because I try and thoughtfully look at what I want to say. I’m absolutely still learning and refining my process, but after a few years I’ve picked up a few tips that I’ll share.
The main thing that you should be watching for is whether or not something will work. Or if something might technically execute, but perhaps it’s making some incorrect assumptions and therefore not doing what your team really needs it to do. In addition, also make sure to keep an eye out for possible regression. Unit tests can help catch some regression, and if you’re ever not sure about something, you can always check out the branch to check yourself. If something is wrong, definitely point it out.
But the way that you point it out is important. You are reviewing the code, not the person, so make sure to be gentle in your wording and strictly refer to the code itself. While it is also an important skill to learn to not take critique on your code personally, it’s also important to make sure that you are commenting appropriately on others’ code and keeping in mind that a lot of thought and work did go into it.
Readability in code is important. It helps ensure that both other current team members and future team members can jump into projects and get up to speed and make changes easily. Sometimes complexity is unavoidable, but if I start reviewing something and it takes me significantly longer than it should to parse through and understand what it’s doing, I will usually see if I can come up with a simpler alternative (even if it takes more lines!) and discuss with the author about whether that approach would be better.
“Any fool can write code that a computer can understand. Good programmers write code that humans can understand.” — Martin Fowler
Readable, simple code is the goal because it makes it easier to maintain, and not just for the author, but for the rest of the team.
Team/project conventions and consistency across the codebase are also important. These are things that similar to readability will help other team members, both current and future, with being able to jump into parts of the codebase that they are less familiar with. If something is being done differently from a defined convention, it should be addressed during review. This isn’t to say that it should be immediately corrected and no one is allowed to propose changes, but a discussion should at least take place about why convention was ignored and if there’s a true benefit to changing that convention.
This is one that I used to struggle with. Coding is a very personal thing, and it’s similar to art in the way that everyone can have their own style. That doesn’t mean that my style is superior to another person’s style though. Just because something isn’t written the exact way that I would write it, unless there is a legitimate reason for me to bring it up, it’s better to let it go.
If my reason for pointing something out was just because I prefer my way of doing it, it’s not worth mentioning. But if something could instead be a learning opportunity for a potentially better way to do something, you should absolutely comment! But make sure that if something isn’t wrong to pose it as a discussion, and just something for the author to consider, not as something that would hold up the merging of the pull request.
Code review can be difficult, and it’s a skill that takes practice to get good at! I’m still learning and adjusting my process, and I don’t think that I will ever stop that refinement of this skill. The biggest thing to keep in mind is that there is a human behind the code you are reviewing, so make sure to be thoughtful and patient as you go through the process.