Code Review#

Code review is a software development practice in which one or more people examine the source code of a computer program, either after implementation or during the development process. The persons performing the checking, excluding the author, are called “reviewers”. At least one reviewer must not be the code’s author.

Code reviews have multiple benefits:

  • Ensures consistency in design and implementation.
  • Can lead to more optimal code (“more eyes on the problem”).
  • Reviewing something as a team reduces the likelihood of misunderstanding a requirement, or design implication.

To the organization, they also provide team benefits:

  • Collaborating and sharing new techniques.
  • Reinforcing mutual understanding of the code.
  • “Collective code ownership”.

When to Review?#

Code reviews should happen after the code is implemented and tested (through automated unit tests). It’s intended to be done prior to merging a feature branch back to main.

Code Review Workflow
Code reviews happen prior to merging to main. Courtesy: https://www.mathworks.com

Performing code reviews isn’t a reason to avoid asking questions earlier in the process! It’s much more helpful to get a second opinion, or clarify a design, earlier in the process.

What to Review?#

Code Review Pyramid
The size of each “slice” represents the importance of that piece when reviewing. Courtesy: https://www.morling.dev

Best Practices#

Save code reviews for major, significant features that have high impact.

  • There’s a lot of overhead to performing these, so you only want code reviews when they add high value.

For lightweight or simple features, consider an informal review.

  • “Can you look at this and tell me if it’s clear?”
  • “Are there any security implications to handling requests this way?”

Pair programming is not a substitute for a formal code review, but can reduce the need to have a formal review in the first place.

Coder#

Document your design

  • Brief summary of the issue you’re solving, and your technical approach.
  • Assumptions that you’ve made; dependencies required.
  • Known issues or limitations with this approach.

Document your code

  • 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).

Book a meeting with them to discuss what they have found.

  • Provide documents to the reviewer ahead of time.
  • Agree on how to proceed, and followup once its done.

Reviewer#

Write down your feedback, ideally as part of the issue comments.

Stay positive and helpful

  • A code-review is not meant to be an opportunity for you to talk.
  • Keep feedback focused on major issues. Save “nitpicking” (e.g. fixing typos in comments) for a summary document.
  • Be respectful of the effort that someone has put in.
  • Never be condescending about someone’s solution.

Code reviews can be a learning opportunity. Look to learn from the code you’re reviewing.

Code Reviews in GitLab#

Code reviews are tied to the Merge Request workflow.

  • When you “Create a Merge Branch”, you select a Reviewer. This is the person responsible for code-reviewing this work.
  • 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.