Learning with Code Reviews

Guidelines for creating a code review experience that enables learning for your teams

Why do Code Reviews?

One of the common reasons cited by teams not incorporating the practice of code reviews is that it slows down development. While it is true that cycles may be added to the current feature, code reviews speeds up development in the long run. Having a readable and well-factored codebase makes it easier to add new features, maintain a high shipping cadence and onboard new engineers. However, the most important benefit is 👇

Code reviews enable active and passive learning — engineers notice best practices, give and receive feedback and build trust with peers.

The importance of code reviews deserves a post on its own, but I’ll save it for another day. The goal of this post is to share guidelines on how best to approach code reviews to make it a fun learning and pleasant experience for everyone involved.

Let’s go!

For the Reviewee

Preparation sets your reviews up for success, reducing turnaround time and making for a pleasant experience for both authors and reviewers.

Hygiene

• No Code should be deployed without a review, regardless of the time of the day, day of the week. NO EXCEPTIONS. If it’s in the middle of the night, wake people up.
• Review your own code before you invite people to review. It sometimes helps to walk away from your screen for a few mins before you submit your pull request (PR) for review. You will spot improvements that will help your overall turnaround time.
• When you are writing 100+ lines of code, it is natural to have a few rounds of feedback. Have a growth mindset about the feedback you receive. Consider feedback as a valuable new perspective that you learn from. You are not Your Code. Mistakes happen and it does not reflect on who you are.
• Take ownership to merging your code. It is your responsibility to address all comments, resolve and merge. It’s okay to disagree, but you should have valid arguments. If you end up taking your discussion offline, leave a comment in gitlab so others know there was resolution. As a bonus, others might learn something from your discussion.
• Always leave the code cleaner than you found it! That means remove commented out code, clean up to-dos, refactor with the goal of improving the code when you touch it.

Commit Messages

Provide clear git commit messages for your PRs. Remember, you are leaving comments for not only your team members but yourself. The context is important when you revisit the commits in the future.

PR Descriptions

Your PR descriptions should be an extension of your commit messages. Provide clear descriptions. Reviewers should be able to understand the crux of the problem being solved by looking at your description.

Preparing your Pull Requests (PRs)

Preparing your PRs the right way can make code reviewing a pleasant experience. Some guidelines to follow:

• Smaller PRs will result in faster turnarounds in reviews. As a rule of thumb, No PR should be more than 250 lines of code. If a PR becomes bigger, break it down into multiple smaller PRs. For example: one PR can have the API definition, and the next one could have the actual implementation. A similar example on the Frontend could be about adding components, and the next one adds logic and integrates with the application. There are exceptions in auto generated code like migrations, boilerplate etc.,
• Big PRs are difficult to review, take a lot of cognitive load and increase turnaround time. Reviewers end up not focusing on the most important aspects like correctness causing bugs to slip through.
• Every PR should pass linting. Linting helps readability and improves velocity in the long run. It also allows reviewers to focus on important aspects like code structure, and correctness.
• Every PR should be accompanied with tests. Test code should be held to the same bar as your production code. Take pride in the work you do, whether it is on the inside or outside.
• Every PR should pass all Unit Tests. Strive for increasing coverage or at a minimum keeping it the same. It does not matter if tests break in another area. It is your responsibility to follow up and have them “all green”. Don’t give up, saying I didn’t touch that part of the code. Sometimes, you’ll uncover unknown dependencies.
• Test your PRs rigorously before requesting a review.
• Leverage the power of Git with Stacked Diffs to not be blocked.

For the Reviewer

• Express appreciation when you review good code. Praising doesn’t cost you anything but will motivate your peers to continue to do good work and improve your relationship.
• Provide constructive and detailed feedback on the code, respectfully. Take the person out of the equation. Formulate feedback as I think, I feel, I suggest, etc., Why? It is hard to argue against someone’s feelings.
• Code Reviews are a great way for reviewers to learn best practices, become familiar with the code base and make changes confidently. Additionally, it is a great way to teach and learn from others.
• Do not accept low quality code even if there have been multiple iterations. It might be tempting to do that in the moment to get on with your tasks, but your future self will not be happy.
• It is okay to take discussions offline, but loops have to be closed on the tool.
• Tests are real code too. Maintain the same bar in reviewing tests. Look at how code can be refactored for setup, reliance on mocks amongst other things.
• Providing quality feedback on code is a great way to build trust and credibility with your team.
• As a code reviewer, you take equal accountability and ownership for the quality of the code.
• Being a great code reviewer is a necessary condition to become an exceptional engineer.

Finally, these guidelines have served my teams well and have been enhanced over time with learnings by practicing them in different settings. What else has worked for you?

Happy reviewing!