A Quick Guide to Peer Code Review and Why You Should Do It
The goal is to make the project better, faster, and more secure through bulletproof code.
After receiving more and more questions about what code review looks like at Netguru, I decided to write this blogpost. As you probably know, we believe that only an honest code review from your work mates brings out the best results.
It’s not about criticizing individual mistakes. It’s about making progress on a project as a team and keeping everything in good shape.
What is code review?
It’s also an opportunity for people to learn good code quickly. For newbies and mid-level developers, it’s a free and accessible way to see how and why things work the way they should. Code review often sparks discussions, where code snippets are passed around and someone always learns something new. Very cool.
To sum it up, code review is not just about the process of review itself, but about learning and teaching others.
Push something to repo where others can see it and suggest better solutions or better gem usage. Learn something from code review and you’ll bring it to another project as well.
I do it every day and it works like a charm!
Why should I care?
Some of you might ask the simple question: why should I care? Well there are a bunch of reasons for doing a code review for you as a software developer, or for the client.
First of all, it reduces huge amount of risk. When you write specs, you can catch some major bugs that might break your app. But you cannot test everything, especially minor changes. Here is where code review comes into play. Checking even small conditions, loops or HAML changes can protect the app from bugs that can turn into larger issues.
Second of all, it allows us to add quick fixes or corrections to our code. If someone caught a wrong indent, comma or space, I would push another commit with fixes and no harm would be done.
Another cool thing is that you can get great feedback from your workmates and other developers. Having code review done is one of the easiest and fastest ways to be evaluated by others. People do a code review and evaluate each other because they know you’ll do the same for them, and that’s how it works.
Gaining kudos is another plus. Writing good code, following best practices won’t definitely be forgotten by experienced developers. Be a hero, and one will show up in your time of need.
‘The happy medium’ rule
There’s a thin line between doing an accurate and inaccurate code review. I call it ‘the happy medium’. I’d say that checking more that 20 commits at once or more than 200 lines does not work for anybody and impossible to do right. It’s proven too, so don’t even try! Conversely, reviewing 1 commit or a couple of lines won’t teach you anything, nor will it give any solid feedback for the commits’ author. The code review matters only when you care, so don’t think about it as a duty, but more like a lesson or fun task.
The culture of feedback
Although most of us are smart, friendly and open for feedback, there are still some people who have an ego. It’s really important to show them that everyone does mistakes and we’re not our code. It’s also practice to not to be a pain in the butt for others. As I mentioned before, code review is not about pointing out mistakes but about learning and teaching. Don’t show how very perceptive you are about people's mistakes. Always try to help and figure out the best solutions for specific case instead of offering only critique. People love constructive criticism, not selfish developers.
The Netguru flow
And now for The Netguru flow. This is how code review works for us, and why we love it.
Every time someone pushes something to master branch the commit gets a ‘pending’ state assigned in our review app . This means a commit needs to be reviewed. We then have 2 office days to ask someone else to evaluate our code. If not reviewed, the commit will auto-reject (expire).
A reviewer can write comments and click either ‘accept’, ‘reject’ or ‘pass’ buttons.
- Passing a commit means that the changes will work properly, but you suggest that it does not follow the best practice. The sql queries might be slow, or for some other reason as a reviewer, you would suggest better solution. Once the commit is passed, the author has another 48 hours to fix the commit.
- A rejected commit means that the changes might have a bad influence on how the app is working and will probably crash the app itself. The rejected commits should be fixed immediately by the author.
The whole process is strictly connected with CircleCI, our continuous integration tool. After running specs, the CircleCI makes an API request to our review app and checks if there are rejected or auto-rejected commits. If so, the whole deployment process is stopped and changes won’t be pushed to the server. This helps us (and forces us) to have our code consistently reviewed. The result is that we can deliver the best in quality to our clients.
To sum it up, we code review helps everyone involved, and it speeds up and secures the development process. Thanks to review processes and reviewers, we catch bugs at very early stages and can focus on making great progress, great products and great developers.
Seems like everyone knows that code review is a valuable tool but a lot of teams struggle with implementing it because of the overhead they feel they just can’t afford right now. Read what we do to keep the overhead minimal at strict peer code review.