Some pointers on code reviews — Part 1
As many other young developers out there, I started my journey with the non-coding aspects of my job ( including code reviews ) on the wrong note. The fact that I was under the mentorship of a coding wizard with a short fuse didn’t help either.
However, moving from simply writing code to fully embracing code reviews was not an easy step for me. As any other starting developer I thought the whole process just a waste of time.
I no longer think that.
During the last couple of years, I have learned a number of good lessons regarding code reviews. I do believe the context or the specifics of my lessons are not relevant to anyone but me. To that end, I will spare you the entire story and move straight to the point.
Here are some things I have learned regarding code reviews.
Consider accountability
I believe a lot of young developers these days take a rather personal approach to coding and code reviews, in general. It’s not hard to see why, really: The project manager assigns me a task to do, I figure out the scope, I do my research, I look at the impact and I make sure I find the best possible approach. So why then, do you get to just come in and “review” something that is clearly well-though and almost perfect ?
Let me ask you this: What if your perfect piece of code fails to perform on a production environment ? What if the 0.001% happens and leads to massive losses both financial as well as in terms of the user’s trust in the system ?
Do you stand solely responsible for that loss ?
That is simply not how this business works. Truth is, (most of the time) the company you work for is accountable. As a general rule of thumb, you should always remember: You are solely responsible for the code you have locally, however, once that reaches a code review, the responsibility shifts to the entire team. Finally, when that amazing piece of code reaches the live environment, it falls under the responsibility of the company itself.
Knowing this, I think it gets a little bit easier to understand why many seasoned developers or team leaders put a lot of emphasis on this process. In many ways, approving a code review is acknowledging the responsibility and accountability for the code being submitted.
Prioritize but don’t overlook
I think we’ve all been there. The team goes through an exceptionally productive sprint and the code reviews keep piling up. The day of the reckoning eventually comes when you have ten, twenty such code reviews to take in. What now ?
Well, first, take a deep breath and strap in. You okay ? Good !
It’s time to prioritize. I know a lot of us tend to give our own code reviews top priority. Well, let’s take a step back, have a look at the bigger picture and start asking some questions.
What seems to be the thing that’s most important for the team ?
What seems to be the highest priority for the project manager ?
What about the customer ?
Keep in mind that it is always a good idea to ask if you’re not certain where to start.
Priority, however, is only half the story here. The other half speaks to overlooking. Overlooking happens when some code reviews are always stuck in the low priority queue. I won’t get into why postponing a code review too much is a bad practice but I will say that nobody wants that. If nothing else, remember this: Low priority doesn’t mean unnecessary !
Look at the code, not at the person
If the title wasn’t clear enough, let me unpack it this quicky. You are part of a team, regardless of being an intern or a senior developer. As part of said team, you and your colleagues are working towards the same goal. We all just really want to write clean code, efficient code and high quality code. However, I think it’s fair to assume that nobody does that all the time.
This is when comments start popping up in your code reviews or, shifting perspective, this is when you start writing feedback on code reviews. Please keep in mind that the emphasis here is placed on “feedback” and “code”.
It’s important to understand the two aspects above. You should not review or evaluate the code based on the person who submitted it. All code must be treated fairly, regardless of it being written by an intern or a senior developer. Try to briefly detach these two entities (the code, the person) and I guarantee you the whole experience will get better.
By doing this, you will manage to avoid two common issues with code reviews. First, we tend to automatically trust the code submitted by our colleagues who seem to have more experience than us — not good. Second, we tend to treat more harshly the code submitted by our colleagues who have less experience than us — also not good.
Self review
Say you just finished a task, tested it one last time and finally opened a code review. It’s time to move on, get a cup of coffee and move to the next feature. Right ? Ehm, hold on for just a second there.
I believe we all noticed that the code inside a code review is slightly different from the one you worked on. First, all of these pretty colors, the carefully arranged indentation and the quick navigation options are not there. Secondly, most of the context of the files, all the stuff that’s around your feature, is not highlighted inside the code review. Lastly, you don’t remember writing so much, do you ?
Well, this is where self review comes into play.
I’m not against coffee breaks, don’t get me wrong. In fact, I think you deserve it. However, when that’s over, let’s have another look at the code and see how it looks like outside your local environment. Does it still make sense ? Do you think the scope is clear to everyone ? Are you sure that going through it won’t give your colleagues a headache ?
There will be code reviews where the answer to all of these questions is “Yes”. How about the other ones, though ?
For all the other ones, there are some steps we can take.
First, don’t be shy to add comments or clarifications on your own code review. You can provide some insights on the scope, ask for special attention to some parts, explain some complex lines and even describe your decision process. It may take you a little bit of time but I assure you, it is well worth it.
Second, attach screenshots or relevant files. This is most useful if you’re working on some UI or frontend features. I think it’s fair to say, most developers don’t have a css or html interpreter embedded in their brain. Having a visual aid to help them nagivate your styling choices can save everyone a lot of time.
Third, make sure you indicate that some files can be omitted from the review process. Most modern programming languages have a lot of stuff pregenerated for you. It could be the actual migration code or a snapshot of the context. It can be the package list or even some csv reports. The code you write should be the target of the review, not the stuff that’s generated from it. Make sure you indicate that clearly as sometimes, it may not be obvious for everyone.
That wasn’t so hard, now, was it ?
Thank you for your time. Part two will be out soon.
Until then, happy coding !
