Code Reviews
A code review is a peer review of code that helps developers ensure or improve the code quality before they merge and ship it. - gitlab.com
Code reviews are typically done as part of your software process, prior to merging code from a feature branch to the main branch.
Why perform code reviews?
When done correctly, code reviews provide benefits to both the project and the team. Let’s consider some of the commonly suggested reasons for code reviews.
Benefits to the Code
There is value in ensuring that code is written to a standard. We definitely want
- Consistency in design and implementation.
- Optimal code (“more eyes on the problem”).
Benefits to the Team
We want collective ownership of our code, so familiarizing the team which a design/implementation is a great idea.
- Collaborating and sharing new techniques.
- Reinforces mutual understanding of the code (team ownership vs. individual ownership).
However, a code review may not be the best format for this. A better approach is to ask for feedback at different points in the cycle e.g., informal review of your design, or a “brown-bag” presentation of your feature as it’s being developed.
Why NOT perform code reviews?
Code reviews are stressful
Code reviews are challenging for both the person who has produced the code, and the reviewer providing feedback.
As the code producer, it’s stressful to have someone critique your work (“maybe I’m terrible at this?!”). This is especially true if the code review consists of being asked to walk through your code, on a projector, in front of your team (who may or may not be eager to be there).
Code reviews are a LOT of work
As the reviewer: code reviews are time-consuming. You need to:
- Understand the problem (i.e. the underlying requirements).
- Understand the design (i.e. “is this the best way to design it?”).
- Review the code well enough to understand the implementation.
Realistically, preparing for even a simple code review is hours of work.
Code reviews are performed too-late in the cycle
Finally, code reviews are usually done at the worst possible time in a development cycle i.e. the end of the iteration! This means that, in-practice, significant feedback from a code review is often ignored (because it’s “too late” and costly to redesign or rework something this late).
As a general practice, you want feedback as early as possible, so that you can act on it. For this reason, we recommend that you adopt strategies that let you get feedback early in the process.
Sometimes teams will claim that inexperienced developers should have all of their code reviewed. This is an incredibly bad idea! It turns out that letting people fail and then correcting them publically
is an incredible inefficient way to provide guidance, and demoralizing to an inexperienced team member.
A better approach would be to use pair programming to provide guidance during feature development, so that you had confidence that the feature was correctly designed and implemented. Better yet if you can get feedback through the development cycle.
When to conduct them
A best-practice is to save code reviews for when they are “needed”.
Here’s some guidance on when to use them.
- Only code review features that are complex, or highly impactful e.g., adding authentication to an application.
- Do not wait until the code review to review the design! Plan to have multiple checkpoints and opportunities for feedback prior to the code review. Treat the code review as the “final pass” (ideally “here is the final version based on your feedback”).
- Code reviews should be rare and there should be no “surprises” from anyone. The outcome of the code review should be (a) shared understanding, (b) small, last minute feedback that was missed earlier in the cycle.
How to conduct them
Given all of the above… here’s some guidance for times when you really need a code review.
For the developer
Document your design
- Write down a summary of the issue you’re solving, and your technical approach.
- Explicitly state any assumptions that you’ve made; list all internal and external dependencies required.
- Explicitly state any known issues or limitations with your approach.
- Make it clear how you are addressing the business need/feature.
Document your code
- Provide written guidance on how the code is structured; what has changed.
- Code comments should only be injected to help readability and understanding.
- Add inline explanations for why something is designed the way that it is.
- Say “no” to formulaic design recipes (unless required to generate documentation).
For the reviewer
Review the materials that the code producer has created!
- This includes reviewing the code ahead of time.
Write a summary of your feedback.
- It’s ok to provide details verbally in the session.
- Be respectful of the effort that someone has put in.
- Never be condescending about someone’s solution.
- Don’t just point out flaws; point out what they did well and what you like about the design!
Code reviews can be a learning opportunity. Look to learn from the code you’re reviewing.
Running the code review session
Book a meeting time. Online (MS teams is fine).
- The code producer and reviewer are required attendees; all other team members are optional. If it’s just the producer/reviewer in attendance, they can walk through the feedback and discuss.
If other team members are present, I’d suggest the following format:
- The code producer presents the design (goals, high-level design).
- The code producer presents the code (ideally, with UML diagrams and not the actual code).
- The reviewer then presents their feedback. If necessary, code can be presented (projector, or shared screen).
- Each suggested change is discussed, and the team decides what changes to make.
Code Reviews in GitLab
Code reviews are tied to the Merge Request workflow.
- When you “Create a Merge Branch”, you select a single Reviewer.
- GitLab (ATM) only supports a single reviewer.
- Reviewer and Coder communicate by adding comments to the issue.
- Coder can close the merge request when any problems have been addressed.
Code reviews are optional in this course. They are available as a tool if you think they are beneficial to your specific project.