Leveling up your code reviews [part II]
The last post about code reviews was entirely from the perspective of the code reviewer. This post is for the other side of the equation — the person requesting a code review. Though of course, if you work in an organization with code reviews, you will switch between both roles all the time.
Here are some best practices for requesting code reviews so that when people see a review request from you, they review it happily.
Pre-code work
If the change you’re making is controversial or there are many different ways to do it, before you even write a line of code, consider a quick whiteboard chat or tech spec with whoever you need buy-in from (your tech lead, the domain expert for that area) to agree on a general approach. This saves a lot of back-and-forth in the actual code review, which is slow and inefficient. If you’re often getting into asynchronous discussions in actual code reviews about the general approach of a large code change, consider doing this.
Be respectful of peoples’ time
Please do not be that person who treats the code review as getting someone else to catch all your style inconsistencies and undeleted debugging code. When you start off at a new company, sure that’s ok, and after that it’s occasionally fine, but you build trust with your teammates by establishing a precedent of improvement and learning over multiple code reviews. So use the “compare branch to master” functionality to review your own code before you open up that pull request. Also, if for some reason, you open a pull request but it’s not ready for review, let the reviewer know. A quick “hold off on reviewing please, still need to make a few changes” shows respect and saves time.
Take feedback well
Code reviews are a great way to get feedback, so thank reviewers if their feedback is helpful. Another part of taking feedback well is not repeating the same issue twice. If someone says “this is the convention, please follow the style guide” make sure you do that on all subsequent changes for the rest of time, until a new convention appears. Show the reviewer that you are appreciative and that you learn over time, and they will be happy.
Keep your changes small
This is not always possible, so it’s hard to put a number-of-lines limit, but the general gist is that smaller changes get reviewed faster and more thoroughly. And then your reviewers are conditioned to expect that reviewing your code is not a burden. Set up your reviewers to not have to work too hard to figure out what’s going on.
Leave context
If you’ve already discussed the change with the reviewer, or it’s one of a series of very similar changes, then they probably don’t need additional context. But if it’s an unexpected change, or there are a lot of other changes that it fits into, it’s super useful to provide a few high-level description or bullet points. “Sets up user endpoint for profile with dummy data and pipes it through to the db.” Context makes it easier to review, which means your code will be merged more quickly. You can also use context to pre-empt anticipated requests: “I plan to refactor this in a separate change, please just review for this added feature.”
Follow through
This is super important for building trust. Sometimes new or junior engineers get caught up when reviewers comment on an approach or request a possibly unrelated change: “While you’re here, can you convert this entire file to this new convention?” It’s hard to know how and when to push back. I’m a strong believer of separation of concerns — muddling together refactoring and adding new feature code in one code change is confusing. But you should also definitely make things better while you’re poking around somewhere. So feel free to say “Yes absolutely, but let me follow up in a separate pull request” but then you must follow through. Otherwise you get in this weird cycle where people know you won’t really follow through so they insist that you do it in that pull request, and then you feel like you’re blocked on some ridiculous refactor, and then you get merge conflicts, and so on. Better yet, clean up or refactor a file to new conventions before you add the new functionality.
So basically, make it easy for someone to review your code, and continue to do so over time. Then they will review it quickly and happily.
After many years working full-time at tech companies, I’m now coaching and consulting engineering teams. If you’d like some help scaling your team or developing yourself as a leader, email me at jean@jeanhsu.com.
