Developers and code reviewers have seemingly conflicting goals: developers want to ship their code fast, reviewers want high code quality. But in my opinion these are two sides of the same goal: keep development velocity high by shipping high quality and maintainable code. Reviewers want to help developers to ship less bugs to production and make code easier to understand so feature improvements can be done faster.
By following these recommendations you’ll be able to reduce number of code review iterations and ship code faster.
Every team has its own standards and best practices. In good teams they are well documented. Reading such documentation in advance will save you many code review iterations.
If you’re new to a team, technologies or an area of the codebase, pairing with someone more experienced could be a great way to learn and complete your task faster.
Split big tasks into several diffs. Just reading 1000 lines of code will take a reviewer more than an hour and a cup of coffee. And each iteration will require another hour. It’s hard to say when a diff becomes too big, but the general rule of thumb is: the less experience you have, the smaller your diffs should be.
By splitting a big task into smaller ones, you’ll get feedback faster, before you’ve spent too much time on a less than ideal implementation. Even better if you discuss, how you’re going to implement a complex feature, with a more experienced developer before you start working.
Before sending your code to review, check it for debug code, commented out code and TODO comments, make sure all kinds of automated tests, type checks and linters are passing. Make sure there are no new lint warnings, and code is formatted, if you’re using a tool like Prettier.
Manual code reviews are great for improving readability, architecture or accessibility. When reviewers are distracted by catching typos and minor issues that could have been caught by automated checks, they’ll be more likely to miss important problems.
Think what may be unclear to a person reading the code, what kind of questions they may have, if they don’t know everything you know about the task. Answer these questions by making code more clear or by adding comments explaining why some code was written and why it was written in a certain way.
If there are obvious simpler alternatives that won’t work for your case, explain why.
These kinds of changes and comments will help everyone who’s going to read the code, not only code reviewers. Good comments may also save your code from accidental “refactoring” that makes code simpler but removes important functionality, when it’s not clear why this “extra” code is needed.
It helps a lot to see what the code in the diff is actually doing. Mention related tickets, add screenshots or even GIFs for all UI changes.
Tip: Kap is a nice free app to record GIFs on macOs.
Code review is a great way of sharing knowledge between developers. It works in both directions: from a reviewer to a developer and from a developer to a reviewer. Code review is a dialog where two, or sometimes more, people are working together on improving a piece of code. To make it work, you need to understand each comment and the reasoning behind it.
Keep asking questions until everything is clear: why something is a problem and what is the proposed solution. Code review comments aren’t always the best place to have long discussions, sometimes it’s better to talk to your reviewer in Slack or even better in person. It’s a good idea to add a short summary of such discussion as a comment to your diff.
Live review can be another way to improve communication. In that case the developer and the reviewer will sit together and go though the code. The reviewer will explain all the problems in details and make sure there’s no misunderstanding.
Reviewers don’t know as much as you do about your task, so some of their suggestions will not make any sense for what you’re doing. There’s no rule that code reviewer is always right, unless it’s a team standard, and even standards have exceptions sometimes. Sometimes suggestions sound good but don’t work in reality.
Use your own judgement, don’t do changes if you see that they aren’t improving the code. Explain why you think that the suggestion doesn’t work for you.
Even if you’ve done the change but don’t like the result, revert it before the next review iteration. A good reviewer will likely notice this too, and asks you to revert the change.
You don’t have to agree with everything code reviewer is saying or to apply all their suggestions, but if you just ignore some comments, a code reviewer will likely write it again.
If you’re not going to do a particular suggestion or have tried and it didn’t work, reply to a review comment with a short explanation to avoid any possible misunderstanding.
Every developer works differently: some will commit all the changes at once, some will commit each change separately. It’s hard for reviewers to know when you’re done with all their feedback and ready for another iteration.
Writing a short comment like “Ready for another iteration” will make it clear, and you’ll get that another iteration sooner.
Development is an iterative process, so is code reviewing. Code review suggestions are possible improvements to the current state of the code, not it’s final state. Often one improvement makes another improvement obvious, and a good code reviewer will let you know about it.
But you don’t have to wait and loose another iteration: do one more self-review and see what could be the next thing.
For example a reviewer has asked you to simplify three functions and make code more consistent. But after you’ve done it, it’s now clear that all three functions are doing the same thing, and you can replace them with a single function.
Don’t treat code reviews as a distraction. They are here to help you ship better code and spend less time on future changes. Help reviewers help you.
Start reviewing someone else’s code yourself, even if you can’t approve it. You’ll improve your own coding skills by reading more code and thinking how to make it better.
Subscribe to my newsletter: https://tinyletter.com/sapegin
Thanks to Anita Kiss, Oliver Joseph Ash, Matt Hamlin, Oleg Isonen, Alex Orange.