Top Seven Reasons I Love Code Reviews

At Snagajob, all code that anyone writes is submitted to the codebase via a Pull Request (PR) on github. New PRs are automatically posted in an appropriate slack channel. Sometimes a specific engineer will be asked to review it, but more often it’s just left out there for anyone to pick up. Everything gets reviewed by another engineer before it’s merged into a mainline branch. (And changes to core libraries are often reviewed by a couple of engineers.) Code is reviewed for bugs, maintainability, readability, performance, coding standards, and sufficient unit tests.

This isn’t something that we did when I first started working here, but I’m so glad we are doing it now. There are great benefits to the individual being reviewed, to the reviewer, and to the organization as a whole. In no particular order, here are some of the reasons I’m excited to be doing code reviews.

1. People catch my silly mistakes. An off-by-one error, failure to handle exceptions properly, a variable that isn’t always initialized. If something like this makes it into production, it might be no big deal — a few extra warnings in the logs that need to be cleaned up. Or it might be major — site errors or data lost. This is one of the main reasons organizations start doing code reviews, and it’s valuable. But there are other benefits that organizations don’t think about going in that are often even more valuable.
2. It helps us establish internal standards and patterns. Snagajob isn’t a huge company, but it’s big enough that engineers don’t all work directly with each other on a daily basis. Sometimes a standard or a pattern will emerge for something we commonly do. But it’s hard to communicate it to everyone. Mentioning it in slack sometimes gets overlooked, or gets forgotten later when one is actually working on a problem where it’s applicable. Our attempts to keep up-to-date documentation have all failed. (At least so far.) But when a second person looks at each change I make, there’s a good chance they’ll tell me about a better way to doing something. And once they tell me, I can use that in all my code, and pass it on if I review the code of someone else who hasn’t gotten the word. Sometimes I learn these patterns and standards from the other side — as a reviewer. I see the similar structures in use a few times, and catch on to it. Either way, it ends up making all our code more aligned. Which means it’s easier to switch teams, or to just help out another team when they are low on resources. It’s also a great way to help someone new to the company learn how we do things.
3. I learn more of our codebase. Anyone can take on anyone else’s PR. This gives me a chance to get familiar with a codebase I haven’t worked in. It gives me a head start if I end up working in it in the future. It gives me a better idea how other bits of code work, which can be helpful if I ever need to integrate with them in any way.
4. Accountability. A reviewer can remind me to write code that’s got more comments, is more readable, has more unit tests, etc. But even before it gets to a reviewer, I take a closer look at all these things because I know it’s going to be reviewed. I’m a lot less likely to tell myself that a unit test isn’t really necessary, or that the code is “good enough” if I am expecting someone else to look at the code and tell me what they think of it before it becomes part of our codebase.
5. It gives me more insight into all the projects and features our engineering department is working on. Even if I don’t review every PR that comes in, I look at the title for most of them. It’s a lot more insight about what other teams are up to than just knowing what high-level projects have been scheduled for this quarter. Sometimes I even see a change coming in that I can really make use of, but otherwise wouldn’t have known about.
6. Getting to see more code gives us more opportunities for code reuse. If I see the same helper function in 2 or 3 different PRs, I can suggest it gets moved into a core library. Or if I go to add code that needs a helper method I’ve already seen, I can move it into that core library myself rather than rewriting it again.
7. It makes the engineering department feel more collaborative. Collaboration one of Snagajob’s Core Values, so it’s something we focus on in a lot of different ways. This is another way to bring it into our day-to-day work. It can feel especially helpful during times where I’m working on a project by myself, rather than as part of a team. I’m still helping other engineers write their best code, and they are helping me write my best code. Teamwork makes the dream work.

Originally published at engineering.snagajob.com on October 12, 2015.