We're proud to be recognized as a Leader in the 2024 Gartner®️ Magic Quadrant™️ for Collaborative Work ManagementGet the report
Becoming a good code reviewer doesn’t come easy, even after almost seven years of writing code. When I joined Asana four months ago, I realized I would need to level up my code review techniques. Here are some code review best practices that are helping me.
Set up a time to talk with your team members about the primary goals of code reviews. If you’re trying to use code reviews to enforce stylistic consistency, but your teammates are counting on your eyes to catch bugs, neither of you will get the most out of a code review. If you all agree on the purpose of the code review you will be able to spend your time more efficiently.
My favorite purposes of code reviews are
To help me learn how to think like my coworkers so that I can more easily navigate and change the code.
To spread knowledge of what files and features have changed recently so that when bugs sneak in, at least two people can help with diagnosing and fixing the problem. No one should ever be alone on a team.
My least favorite purposes of code reviews are
Catching bugs. I think automated tests and using the app are much better ways to find out how code actually runs.
Enforcing basic style rules. I think linters are much better for this. Why waste a human’s time on something a computer does better? Plus, getting a bunch of negative and unimportant comments can undermine feelings of teamness and trust.
Reading code is a pretty unnatural way to interact with code. Finding bugs just by reading takes years of practice. The computer is much better at running code than your head is.
Chances are, if you run the app, you will do something slightly different than what the author tried when testing their change. You may discover important cases they missed.
Setting breakpoints in running code can be very informative when dealing with code that has a complicated life cycle. Running the code and hitting three breakpoints might save you half an hour of trying to understand how pieces fit together. Plus, if the code follows a different life cycle than you expected, you have probably found a bug or learned something important about your application.
I often find myself spacing out or glossing over details. I need tricks to force my brain to absorb the code more deeply. Sometimes drawing or visualizing which methods call which other methods, or which objects use which other objects helps. The key is quizzing yourself. Reading is not as good as extracting something from your own memory and writing it down. You’ll recall the change better later if you test yourself on it now.
Even if it looks like a large review, try to make a first pass as soon as possible. Your coworkers will be grateful, and their goodwill will help you grow faster as a reviewer.
It isn’t always easy to do code reviews immediately, especially if they change a lot of code or it takes a long time to start up your app. If you find yourself procrastinating on code reviews these tips might help you get started faster:
Set a time limit, like half an hour. On your first pass, spend that half hour trying to understand the change and writing down questions. If at the end of the time limit you think you can approve the change, approve the change. If you aren’t ready, send the author any thoughts or questions you have so far, and schedule and commit to a time when you intend to do a more detailed pass and approve or request changes.
Keep two separate repositories on your machine, one for your own changes and one for changes you are reviewing. This way, compiling your coworkers’ changes won’t destroy compile artifacts associated with your own changes.
Read the feature description first, and try to make a list of the files you expect to change. Then, review the files the author actually changed. If they’re different than what you expected, figure out why. Some files may have been changed by accident, or there might be a chance to learn about the code base. Testing yourself like this is a good way to practice for implementing similar features.
Phabricator and GitHub are mostly only optimized for showing text differences. Editors like App Code, IntelliJ, and RubyMine are optimized for showing code (I really love the Jetbrains code-diff tool built into the IDE).
Pull the change onto your own machine so that you can see compile errors, warnings, and test failures. This will also give you the tools you normally use to browse code, like command-clicking and searching for usages. Reviewing an entire file — and not restricting your sight to the lines that change — will help you see if related pieces of code are spread too far apart in a file and if the overall layout of the file will be approachable for future programmers.
When you suggest changes, assume that the author can handle the changes. Slowing them down to wait for a second round of code review is almost never worth it, especially if the changes are simple things like renaming variables or extracting a duplicated method.
If you make authors wait longer because of style, the code will get worse in the long run. Slowing down changes makes people reluctant to submit small changes that clarify and clean the code.
If you don’t feel qualified to give approval, say it in those words and come up with a plan to get the right person to look at the code. When you forget to click “approve” the author of a change doesn’t know if you forgot, or you think the code is broken, or you don’t care if they are blocked. Help them feel your good intentions by being very clear about what the next step is.
Hopefully, these strategies have given you and your team some conversation starters on how to do code reviews. If you’re interested in learning more about our team, engineering best practices, or open positions, check out our engineering team page. Happy code reviewing!