What is a Code Review?
A good code review is like a good home inspection. It ensures that the original architectural drawings were followed, verifies that the individual subsystems are built and integrated correctly, and certifies that the final product works and adheres to standards. In the world of software development, a “code inspection” often takes the form of a pull request. A pull request, by its nature, simply shows line by line information for all of the updated or added files. This allows developers, who didn't write the code, to perform a code review and approve the changes before they are merged into the codebase.
Why Code Reviews are Important
Code reviews are not just a quality assurance (QA) process with a different name. They offer a wide variety of benefits that go well beyond the inherent advantages of a QA process.
Training / Mentorship
Deadlines often mean that time is not set aside for mentoring developers, or when it is, finding scenarios to illustrate various ideas are hard to find. Code reviews offer a great way to organically integrate training and mentorship into everyday workflows. It allows developers to learn from each other and leverage the years of experience from everyone involved. Senior developers are able to impart background knowledge of past decisions and provide guidance on a variety of best practices.
Knowing that another developer will be looking over your changes line by line forces you to think more deeply about your code as you write it. Instead of thinking “what is the fastest way to accomplish this task,” you are forced to think about the right way to build functionality. Your end goal is code that not only fulfills the scope of the ticket, but that is easy for others to understand and review.
Sharing the Load / Burning Down the Silos
Software development often requires specialization. When everyone’s work is visible in a pull request, it offers others the chance to see how unfamiliar problems are solved. This allows everyone to ask questions that help others, encourage additional learning, and diversify developers’ knowledge. Also, when things do break, more members of the team will have know-how to quickly track down the bug.
Peer reviews help ensure that any code that is committed to the codebase is not only functional, but meets quality standards. Typically, this means that it:
- Has good documentation.
- Meets code style guidelines and best practices.
- Is performant.
- Doesn't add unnecessary lines (uses existing APIs).
- Properly abstracts code where appropriate (adheres to D.R.Y. principles).
- Is future-proofed as well as possible.
Senior developers may know about needs for future planned functionality or the needs of another internal team and can help steer current architecture and code changes to support those efforts. Some additional work now may pay huge dividends in the future by simplifying the addition of new features and maintaining existing functionality.
A peer review requires a pull request that is able to be comprehended by another developer. By introducing a peer review workflow, pull requests often decrease in size to aid the review process. This reduces risk, as often this relies upon smaller more discreet change sets instead of monolithic changes in a single pull request. Additionally, by having developers with varying areas of expertise, potential security risks are reduced by catching issues during the peer review process instead of after they’ve made it out to production.
How to Do a Code Review
Creating a Pull Request
For a peer review process to reach its full potential, it requires a skillful reviewer, but also a well-prepared pull request. A pull request might receive many reviews, but the first review it should receive is from its author. When preparing your own pull request for review by others, there are many things that can be done. These include:
- Provide context for the changes by including a summary.
- Document outstanding questions/blockers if further progress is pending feedback.
- If necessary, clear up any potential areas of confusion ahead of time with preemptive comments.
- Realize that if confusion could exist, perhaps the code or the code comments need improvement.
- Attach screenshots of the functionality in action to provide additional context.
- Ensure reviewers are aware of the state of your pull request and are notified when their review is needed.
Reviewing a Pull Request
Providing feedback on a pull request is a lot like coaching, it requires building trust, adjusting to the situation and the player, and treating everything as a teachable moment. Let’s walk through some of Chromatic’s tried and true techniques.
- Cite requests with links to posts, code standards guidelines, etc. when pointing out lesser known things or to build rapport with a developer not aware of your expertise.
- Ask leading questions that help other developers see things from your perspective instead of just saying "change this."
- Keep it fun and where possible, introduce some self-deprecating humor. Bring up your own past mistakes as evidence for why an alternate approach should be considered.
- Acknowledge when you are nitpicking to make feedback more palatable.
- Avoid pointing out problems without providing solutions.
- Denote a few instances of a repeated mistake or enough to establish a pattern, then request that they all be fixed.
- Ask questions. Doing so allows you to learn something and the author is forced to think critically about their decisions.
- Explain the why of your requested change, not just the what.
- Avoid directing negativity at the developer, instead focus on how the code could be improved.
- Enforce the use of a code sniffer to prevent wasting time nitpicking code style.
- Make note of the good parts of the code code and celebrate when you or others learn new techniques through the process.
- Avoid bikeshedding on trivial issues.
- Realize when optimization is no longer useful and avoid over engineering solutions.
- Remember that the goal isn’t to find fault, it is to improve the codebase and teach others along the way.
- Written communication may not come off in the same tone as it was intended, so error on the side of friendliness.
- Treat others how you want to be treated. This is a simple rule, but such an important one. Consider how you would feel receiving this review, and be sure to write your review accordingly.
Establishing a Workflow
A pull request won’t merge itself, request feedback, or implement the feedback it was given. Thus, having clear expectations for who creates, updates, reviews, and merges a pull request is crucial. For example, after a pull request has been created, is it the responsibility of the developer to solicit feedback until it is approved and merge it themselves, or does a development manager take ownership of every pull request and merge it when they deem it ready? A good workflow should have just enough rules to work, without overly burdening developers and will likely vary per project or per team. The correct workflow is the one that works for you, the important part is that you have one, and that it includes a code review.
Adding a code review step to your development workflow might slow things down at first. However, in short order it will invariably show itself to be a worthwhile investment. Code reviews are an invaluable tool for mentoring, improving accountability, sharing knowledge and enhancing overall code quality. Now that you have reviewed this post, if you approve it, then merge it into your team’s workflow.
The bad puns are done, but if you want to continue the discussion on code reviews, reach out to us on Twitter @chromatichq.