Non-blocking Code Reviews with Github PRs

In this post I want to demonstrate a simple post-merge foreman style code review flow that can be implemented using GitHub repositories and Pull Requests (PR). While it’s not full-fledged trunk based development, it can be pretty close for those who are not yet ready to take the no-branches leap.

Before I jump into it, let me present you with several reasons for why you could want to adopt a non-blocking code review workflow. If you’re familiar with foreman or TBD practices, you can skip to the GitHub setup.

Code Review != Pull Request

There are many ways to review code:

• When you write new code, you’re reviewing the existing codebase.
• If you join a colleague for a pairing session, one of you is coding, the other one is reviewing.
• As you read new commits on master with your morning coffee, you’re reviewing code.

The science on the matter is clear. Code review works—it does make the code more robust, high quality, and less polluted with bugs. However, the review method is not as important as the mere fact of doing it.

Code review doesn’t need to be blocking

Nobody likes waiting for code review. Numerous books and experts urge us to perform code reviews many times per day so that nobody is waiting for more than 1-2 hours. In reality, this is next to impossible to achieve. I’ve never seen a team consistently deliver code reviews at such a short cadence. To shorten the wait time to 1-2 hours, developers must context-switch every 1-2 hours. This isn’t the most productive thing to do in an asynchronous team.

There is another way. Instead of pre-integration code review you can use post-integration code review, referred to as foreman (well explained by Uncle Bob).

Foreman code reviews work like this:

• Trust everyone to push straight to trunk.
• Review each new commit on trunk at a convenient time.
• Push post-review fixes to trunk (and review them).

Foreman is not for everyone

A word of warning, though. If your team commits code that breaks production, and you don’t have a test suite robust enough to catch these bugs, be careful with this approach. While it enables shorter cycle times and higher developer happiness, it produces additional risks considering stability and reliability of your system. However, for a high-trust, disciplined team, this trade-off should be an obvious improvement (unless you’re NASA).

Before switching to this flow consider whether the following points describe your team:

• Every developer can run the full test suite on their local machine.
• When the build breaks, you pull the andon cord and storm the problem and fix it quickly.
• Your CI/CD pipeline is quick (< 10 minutes) so that any hotfix can be quickly pushed forward.
• You can roll back to a previous release in minutes.

GitHub setup for foreman code reviews

To create an effective foreman code review system, we need to:

• List all the changes pushed to trunk that need to be reviewed.
• Allow accepting or rejecting each of the changes.
• Enable discussion around the code.

Below I present how it can be implemented in GitHub using simple constructs: PRs and GitHub Projects.

Use regular Pull Requests

Push your changes as regular Pull Requests, then merge to trunk. This will allow you to track each change set later on, and provide a space for code review. You can utilise GitHub’s great CI/CD capabilities, either with GitHub Actions or any other CI/CD provider. Moreover, check out GitHub’s auto-merge feature—it’s super useful.

Create a Code Review Board project

Create a GitHub Project for tracking newly added changes. It’ll serve us as a kanban board for code reviews. Using GitHub Projects’ automations feature we can configure the project so that every merged Pull Request automatically appears in a Pending Review column.

From now on, developers review this list at their convenience. They move each PR to either accepted or rejected column.

Now developers can review each other’s code in a non-blocking, asynchronous manner.

Use PR code review mechanism to discuss rejected PRs

With the above workflow, once the author merges a PR, they can’t push more commits to it after the code review. How do we commit post-review fixes then?

It’s helpful to establish a branch naming convention here. For example, when a reviewer rejects a branch named add-payments-module, the developer pushes the follow-up changes to add-payuments-module.fix-1 branch. Then, the developer can reference the original PR and assign the reviewer for extra better clarity.

Stay flexible — sometimes waiting for CR is good

This workflow doesn’t rule out pre-integration code reviews. Sometimes, it might make more sense to wait for a code review before merging. For example when:

• A junior dev is not yet confident enough to merge on their own.
• A developer introduces a critical, risky change to the system.

The post-merge review should be the default, with the option to ask for a review before merging the changes.

Summary and possible improvements

I think the foreman approach to code reviews is an interesting alternative to the broadly accepted pre-integration code review flow. Our jobs as programmers is to build exciting and useful technology for our customers. Given the complexity of what we do, we should streamline the process of delivering software as much as possible. Shipping fast is important to developer happiness. And if developers are happy, they do their best job, making the customers happy.

Disclaimer: I have not used this method in any team yet. Check out my GitHub repository where I demonstrate this workflow. Perhaps it’ll give you more insight into whether that’s something that would work in your team or not.

Below I list several tweaks that could further improve the above setup.

Using labels

To make PRs searchable using GitHub UI and filter rules, we can introduce 2 labels:

• accepted
• rejected

When a developer merges a new PR, it has no label. Then, the reviewer assigns a proper label post-review.

Better Projects automations

GitHub Projects’ automations feature is limited. Each PR requires manually adding to the project. Label-related automations are not available.

To solve the above problems (reduce the toil of manual work), we can use project bot. It expands on Projects automations idea with more automation possibilities.

Here are rules I’ve created with project-bot for the foreman code review flow:

• Assign new PRs to the New column of the project automatically.
• When merged, PRs go into Pending Review column.
• From now, the reviewer adds rejected or accepted label:
  • Adding accepted label moves the card into Accepted column.
  • Adding rejected label moves the card into Rejected column.

This could be automated using GitHub Actions. The GitHub API supports manipulating PR’s labels, projects, and other metadata. If you know a Github Action for Projects automations that could be used for foreman code reviews, let me know!

I hope this post proves, if anything, that workflows alternative to classic PR-review-merge flow are broadly available and easy to implement. As with choosing the right technology for a problem, we should explore various workflows and use the right one for our particular team and business context.