Recently at CodeHS, we’ve put a much bigger emphasis on our code review process. This is mostly related to our team growing. Before, I personally had the bandwidth to code review all of the code that was written. Now, with a few more engineers on the team, it’s just no longer possible.
It’s also because I believe there is no better way to learn and improve as a programmer than with code review. Ok, maybe pair programming, but for pretty similar reasons so I’m just going with it.
There is no better way to learn and improve as a programmer than with code review.
We’ve also instituted a new process for integrating code review into our engineers’ daily workflow without causing it to get in the way.
What we’ve done in the past
We used to have people do code review on occasion, when they had time, or when I asked them to do it. This was often a problem for a few reasons:
- Engineers are busy and breaking up their day to focus on code review often broke up flow on their own projects.
- They often weren’t familiar with the project they were reviewing, so it took even more time to understand the project before they could give a quality review. Or, the review they gave was limited in quality because of the lack of context.
- Code review didn’t happen immediately, so engineers often had to wait a long time before getting review on their project, causing them to linger.
Our new process
Every day, every engineer reviews one pull request first thing in the morning.
Why does this work better?
- It becomes much clearer how much code review you’re expected to do as an engineer, so you know how much to do review and how much to work on your own projects.
- Doing review first thing in the morning means you finish it before getting back into your own projects. This lets you focus on your projects completely for the whole rest of the day, knowing you won’t get pulled into code review at any point until the next day.
- It’s consistent and frequent. Reviewing every day ensures that you are practicing often. You now review code in parts of the codebase you’re less familiar with because you are reviewing so much more often.
- It’s tightening the feedback loop on your code. Catching flaws in the design of a project earlier makes it much easier to fix, so now we are much more capable of giving people review early on in projects rather than mostly just at the end.
If there aren’t many open pull requests on a given day to review, you can review a recently merged pull request that you hadn’t reviewed. You may catch things that weren’t caught that could still improve. At least, you’ll be more familiar with the changes on the site and be more prepared to jump into that area of the code if you need to.
So far, this new process has improved our team’s productivity by having a quicker turnaround time from pull request to review. It has also increased the number of places in the codebase our engineers understand. Both great things.
Aside from making the team more productive, code review makes you a better programmer. Here are a few tips for becoming a master at code review.
Learn by reading others’ code
When reviewing someone else’s code, you get a chance to learn from it. You should work to understand what they’re trying to do and how they do it.
Ask questions about how things are working or why people made certain decisions. Even if you think the code is good, there is still an opportunity for you to learn. Not every comment you make has to be constructive feedback. This is as much an opportunity for you to grow as it is for the person you’re reviewing to get feedback.
Code review yourself
Don’t just code review others. Before you submit code for review, make sure you’ve reviewed it yourself. Scrutinize it as if it were someone else’s. What could be clearer? Where could you leave extra comments? Are your variable names and function names the best they could be?
Learn what questions people ask in code review and ask yourself those questions.
Code reviewing yourself can reduce the number of cycles of review and save both you and other engineers time. Make it a habit.
Make a mental (or written) checklist of what you look for
When I code review, in addition to looking at the overall big picture of the pull request, a few things are always circling in my mind that I always make sure to look for. Here are a few of them:
- Is every variable named clearly? Does the name make sense? Does it match the correct singular vs. plural?
- Are the function names good? From reading just the function name, can you correctly guess the return type? Is it clear if there are side effects?
- Are the urls named well? Will they make sense in the hierarchy you have on your site? Will users understand them?
- Are there comments everywhere? We also use a linter to make sure comments exist, but I actually read the comments and make sure they’re helpful
- Can a function be decomped?
- Anything else you see often
Be thankful for the code review you receive
Think of receiving code review as a private tutoring session from another engineer.
Never take code review personally. The goal is to make sure that the code that’s going into the live codebase does the job and is as good as it can be. This is also a fantastic opportunity for you to improve.
Think of receiving code review as a private tutoring session from another engineer. They put time into trying to make your code better. Take advantage of that and learn from it. Learn what their approach is and work to not make the same mistakes across projects. That feedback loop is how you improve.
Thank people for review. They’ll appreciate it and want to continue giving good review when they know you value it.
Don’t just review the code. Test it out
It’s such an easy trap to fall into. Don’t do it.
You gain so much more insight into the project when you actually try it out locally and try to break it. While code quality is very important, it’s completely unhelpful if the code doesn’t work as intended.
Scrutinize like a beginner
Particularly when testing out code, you should comment whatever you’re thinking. It’s like free user testing for the product, which is really valuable.
Our team is currently doing a training exercise to practice scrutinizing features built when code reviewing. We’ve looked at http://www.useronboard.com/ and are studying the observations they make about onboarding workflows to train ourselves to know what to look for. They do a really good job of questioning the designs, layouts, and decisions the sites and apps have made in their organization.
After reading through a few of the teardowns on the site, each of our engineers will create their own teardown of a part of CodeHS they aren’t super familiar with. The goal of this is to make sure you’re noticing everything as you test a feature. If you train your eye to notice things, you have an opportunity to catch problems well before they go live, so you can spare your whole team from those issues reaching customers.
It’s also helpful to review code and test through the eyes of a beginner. Remember, the person using the end product will be a beginner. But also, the next person working on this part of the codebase will also probably be a beginner, at least to that part of the codebase. Help them out in advance if you notice something that may be hard to follow if you don’t have as much context.
I hope this was a helpful piece that encourages you to put even more emphasis on code review. I really believe it’s the key to improvement, both individually and as a team.