How to do pull request reviews

TLDR

Having a process, even a simple one, for creating, reviewing, and merging PRs (pull requests) for all of your repositories is a necessity when working with a team. It makes the software development process more efficient because all team members know the requirements for getting code pushed to production and they can collaborate effectively to make it happen.

Outline

• Code repository hosting service
• Branching
• Creating the PR
• Reviewing the PR
• Merging the PR

Code repository hosting service

Github versus Bitbucket

Two of the most popular services for hosting teams’ code repositories are?Github?and?Bitbucket. Check the?features?and?pricing?for Github and the?features?and?pricing?for Bitbucket to see which one is more suitable for your team. If your team is already immersed in the?Atlassian?ecosystem (e.g. using?Jira?for their project management software and?Confluence?for their documentation), it makes sense to also use Bitbucket because it integrates well with Jira.

What we use at Mage

We use Github to host our repositories. It’s hands down the most popular hosting service used by developers and easy to onboard new team members since they are probably already familiar with it. The Github Team pricing plan is sufficient for a team of less than 10 developers.

Branching

• Mage’s branch-naming convention (without brackets): [initials_or_team_name] — [your_branch_title_snake_case].
• Prefix commit messages with your initials in brackets:?[initials].

$ git checkout -b [initials_or_team_name] — [your_branch_title_snake_case]

$ git checkout -b jk — new_sign_up_flow
< make some code changes >
$ git add .
$ git commit -m ‘[initials] <title of your changes>’        

Creating the PR

These are the requirements for team members creating new PRs:

• Add at least one reviewer (adding multiple reviewers may not be allowed in Github’s?free tier).
• Assign yourself and anyone else who worked on the PR as assignees.
• Include a brief summary of what was done. This could be bullet points.
• Include screenshots of the new features if applicable (e.g. adding a feature to the UI on the frontend).
• Include sample API request payloads if applicable. It will be easier for the reviewer to check that the payload is formed correctly.
• Mention any tests that were added or performed.
• Prefix the title of the PR with your initials (e.g.?[jk]) so that the initials appear in the commit message in the?main?branch’s commit history after the current branch is merged.

Reviewing the PR

The reviewer goes through the changes and makes any comments as needed. There may also be some non-blocking comments (e.g. minor comments that don’t require any fixes) that the assignee should be aware of but don’t require further action. Then the assignee addresses all of the comments and informs the reviewer that the PR is ready for a follow-up review. Once the PR is ready to be merged to the?main?branch, the reviewer approves the PR.

Ready to ship that code. (Source:?Giphy)

Merging the PR

An approval from a reviewer is required before being able to merge the PR. Github can be configured so that an assignee cannot actually merge the PR without that approval (the merge button will be disabled).

Before merging the PR to the?main?branch, there should be no conflicts with it, and all tests as part of the?CI/CD pipeline?should be passing on the current branch. We’ve written a separate?blog post?on how we set up our CI/CD pipeline.

There are?3 ways?to merge a branch in Github:

1. Merge commit — All commits from the current branch are added to the base branch, and the PR is merged using the?— no-ff?option.
2. Squash and merge — The PR’s commits are squashed into a single commit, and the PR is merged into the base branch using the?fast-forward option.
3. Rebase and merge — All commits from the current branch are added onto the base branch individually, and the PR is merged using the?fast-forward option.

We use the “squash and merge” method for merging the current branch into the?main?branch. This can be configured to be required in the Github repository?settings. We prefer to squash commits because it keeps the?main?branch’s commit history cleaner and easier to read.

After the current branch is merged into?main, delete the current branch. This can be done automatically in?Github.