Code Reviews

At Nimble, code is never merged directly into the primary branches like main, develop, or release. Each feature, bug, or chore branch requires code review before proceeding.

A code review is a software quality assurance activity where several people read the source code changes and give feedback. The goal is to improve the code quality and ensure that the code is well-designed, follows best practices, and delivers the required functionality.

All code reviews are managed through pull requests (abbreviated as PRs). The people performing the code checks on the pull requests are called code reviewers. Once satisfied with the code, a code reviewer approves the pull request. The code can be merged when it has enough approvals from the reviewers.

All parties involved in the code review process – the pull request’s author, code reviewers, and the code merger (usually the Team Lead) – must have clear guidelines and efficient practices to ensure the code review process benefits a project.

Standard Cycle

A pull request goes through four standard stages: draft, ready for review, in review, and merged.

Draft

• The author opens a pull request with the draft status. This status indicates that the pull request is still in progress, i.e., new commits are still being pushed.
  • On GitHub and GitLab, open pull requests in draft mode since both version control providers have built-in support for this.
  • On Bitbucket or any provider not offering draft pull requests, add the prefix WIP to the pull request title (e.g. WIP - [<User Story ID>] As a user, I can search for articles).

• Code reviewers can start reviewing draft pull requests, despite no review request has been sent, but must NOT approve them until the author marks them as ready for review.

• Open a draft pull request as early as possible to:
  • Get early feedback on the implementation. The Team Lead can detect issues early in the implementation choices hence rerouting efforts earlier in the sprint.
  • Share the code with the team. Other squad members can cherry-pick code from the branch, even though it is not complete, to make progress on their own tasks.

Ready for Review

• The author marks the pull request as ready for review. This status indicates that the pull request is no longer being worked on, i.e., no further commits will be made to the branch.

• The pull request description must have all the required information for the code reviewers to understand the changes.

  Learn more about best practices to structure a pull request

• The author moves the associated user story to the In Code Review column on the project board.

• The author assigns reviewers to the pull request. While the author can notify the reviewers through Slack, the pull request’s assignees are responsible for organizing themselves to review the pull request timely.

In Review

• The code reviewers review the pull request and provide feedback.

  Learn more about best practices to provide feedback

• The author addresses the feedback and pushes new commits to the branch. The reviewers are notified of the new commits and can review the changes in subsequent code review batches.

• Depending on the scope of the changes, the author should consider changing the pull request’s status:

  • For minor changes, the author can push the changes to the branch and keep the pull request status as Ready for Review.
  • For major changes, the author can mark the pull request as Draft to indicate that the pull request will be updated with significant changes, hence will require a fresh review.

Merged

• A pull request is only merged when the squad has confidence in the code and its impact on the project.

• Merging requires a minimum of two review approvals:
  • In small squads (two developers or less), the approvals must include the other squad member and an external code reviewer (i.e., a team member assigned to the project as a code reviewer but not as a developer).
  • On large squads (two developers or more), the approvals must include at least the Team Lead and/or one senior teammate to avoid merges with approvals from only junior teammates.

• A single squad member has the permission to merge pull requests. This person is usually the Team Lead, but it can be temporarily delegated to another squad member.

• The code merger moves the associated user story to the Ready for QA column on the project board.

Best Practices

Self-code Review

A pre-requisite before assigning code reviewers is to do a self-code review as it provides the following benefits:

• Catch numerous trivial issues such as common code convention mistakes and typos, thus reducing the feedback workload for code reviewers.
• Take a step back and look at the code from a different perspective, which can lead to better design/implemention choices.

Timely Feedback

When code reviews are slow, several things happen:

• The velocity of the team as a whole is decreased.
• Code health can be impacted.

To avoid this, the team has the following expectations:

• Code Reviewers should review pull requests shortly after they come in if they are not in the middle of a focused task.
• One business day is the maximum time it should take to respond to a code review request (i.e., the first thing the next morning).

Team Orchestration

In a squad, with numerous developers, the number of pull requests can be overwhelming. Leaving code reviewers to decide which pull request to review can lead to a situation where the most important pull requests are not reviewed first.

To ensure the code review process is efficient, the following practices are recommended:

• Add priority labels on to pull requests:

  

• Provide a list of pull requests sorted by order to review to the code reviewers on Slack.

Code Reviewers Notification

GitHub, GitLab, or Bitbucket will automatically notify the code reviewers about the pull request, the author of the pull request does NOT need to ask for code reviewing immediately after creating the pull request.

However, notifying the code reviewers on Slack is recommended in the following cases:

• No activities on a pull request after a half-day or full-day.

  

• Urgent features or issues must be merged.

Speedy Merges

• Merge PRs within 2-3 days of their creation to ensure an efficient Cycle Time.

  Stale PRs are usually hard to merge past this time range and require tedious rebase.

• Refocus the squad’s efforts on code review if too many pull requests are waiting to be merged.

  Open PRs represent work that is much closer to being done than new work. The squad should focus on finishing the work that is already in progress before starting new work.