DEV Community

Nathan Mattes
Nathan Mattes

Posted on • Originally published at zeitschlag.net on

Some Thoughts on Code Review

A few days ago, Max and me recorded a new episode of the German Codestammtisch-podcast. We talked about code review and pull requests. In the last couple of days I witnessed again, how useful these two can be. The full episode will air on March 22nd — I still have to produce it.

Half a year ago, my team moved from Subversion to git. And we used that opportunity to introduce code reviews and pull requests as well.

They do not only serve as a tool to spread knowledge accross the team and increase the bus factor, but they also serve as a safety net: They've helped us to catch a lot of these tiny quick-fix bugs I mentioned in my last article, especially last week. These things happen easily, if your mind is somewhere else when you work, but hey, code review is here to save the day!

Last week was a bit stressful in the office. As a team of three, we're handling multiple projects at the same time. Although it's not a good idea in general to tackle more than one thing at once, it's not possible to fully concentrate on only one of these projects, unfortunately. And therefore it's a bit stressful at the moment.

But we don't want to get rid of code reviews, as they've turned out to be incredibly useful for us. Even if that means that we have to switch projects multiple times a day, it's totally worth the effort. The earlier we catch bugs, the better (and cheaper). Despite the stress, I consider it crucial to review code thoroughly, but respectfully:

  • Whenever I find a code change I appreciate, I take the time to write that down and thank the author.
  • If I don't see or understand the reason for a change, I just ask, so that the author can explain it to me. Before I suspect a mistake, I want to make sure, that I didn't miss something.
  • And if I find a tiny bug that slipped in, I respectfully ask the author to fix it, too.

Nice things don't take a lot of time, are mostly free and (hopefully) improve everybody's mood.

A few days ago, I read a German article on heise.de about pull requests. It came to my mind then, that we didn't talk about the task of a reviewer. Should they only do the code review? Should they run and test the app as well?

In the end we came to the conclusion, that a thorough code review and a short manual test should be sufficient for us. But nevertheless, we talked about it and killed another assumption. Feels good!

Top comments (1)

Collapse
 
jessekphillips profile image
Jesse Phillips

In a code base you're not familiar with it can be had to do a code review. Your suggestion to ask why is a great way to get the author to talk, it will help them consider the situation and he may say something you understand.

I also recommend doing a code review yourself before making the merge request.