Improve Your Code Review Process

Like many other teams, there will always be bottlenecks in the development process. But your Code Review process shouldn't be one of them.

⚠️ Problem: Poor communication of Code Review (Pull Requests).

I've been on teams that use instant messengers to communicate when a PR is ready for review. Utilising emojis to give status updates.

👀 - reviewing

🔀 - changes requested

✅ - approved

📝 - comments left

Using instant messaging for updates on PRs is an ineffective way of communicating around PRs for the following reasons:

• Requires colleagues to have notifications enabled for emoji reactions.

• You have to go hunting for channel posts to find the PR you're reviewing/authored.

• The PR channel can fill up with larger teams, meaning PRs are often missed.

As a result, PRs begin to pile up, and block work (even if you adopt an efficient user story strategy) until the PM (Project Manager) begins to ask questions.

Another poor method I've seen is instant messaging people, with managers asking the team members to:

Find your reviewers / take responsibility for your PR

The thinking behind this is to take ticket ownership off the rest of the team and managers chasing up the status of the PR.

Yet, in practice, this doesn't happen. Instead, what happens is that colleagues team up and go back and forth reviewing each other's work (meaning other team members need to be exposed to the code base, creating knowledge gaps). Or, people in the team find the person who's great at reviewing.

Solutions

You can solve this problem with project management tools such as JIRA, Trello or many others available online (paid & free). Use this as a source of truth for your dev team workload.

Taking the example of JIRA or Trello, both work on visualising the status of tickets (work) that needs to be completed within the team.

My suggestion is to have some basic initial columns on your Trello / JIRA board, such as:

• Todo: A column where unstarted tasks wait until a team member becomes available.

• In Progress: A column for where team members work on and complete their assigned tasks before moving them to the next stage."

• Awaiting Review: Tickets move to the "Awaiting Review" column once the developer finishes working on them and creates a Pull Request for review by one or more team members. Add the Pull Request link to the ticket to make it easy to find.

Using project management boards promotes teamwork, ensuring that all work is reviewed and ready for testing. It's everyone's responsibility to make sure the team meet their goals. By doing so, we can avoid burdening the more productive team members with the task of chasing PRs. No more cluttered messaging channels filled with links, emojis, and missed messages.

It also means each member of the team has the opportunity to get involved with the code review process (which, in my opinion, offers far more than just code quality) by simply taking a look at the board and seeing if there's anything in the Awaiting Review column, not being looked at by another member.

⚠️ Problem 2: But Who's Reviewing My Pull Request?

So we've solved the issue of knowing if there is a Pull Request to be reviewed, but what about whether someone is looking at it?

Solution:

Your board can have indicators of who is working on it. For example, using Trello, you can add Members to the board, indicating that this member is associated with the ticket in its current state. Likewise, in JIRA, you can assign the ticket to yourself, or others in the same way.

Jira Example:

Trello Example:

Note: When adopting this strategy, I suggest that you agree when moving items into the Awaiting Review column that the assignee is set to unassigned, meaning that it's clear that no one is reviewing it. Then, when you're picking up the PR, you should assign your name to the ticket so it's someone is looking at it already.

Alternative option

An additional option is to utilise the built-in functionality of many Git hosts for assigning users to Pull Requests. For example, take GitHub (one of the most popular git hosts). You can use their assignees functionality to assign yourself or other team members to a created pull request.

Why will this help?

The team could use GitHub as the source of truth; Github becomes the one place where all Pull Request activity is monitored and recorded.

Team members can easily track pending pull requests and their review status from the Pull Requests screen in GitHub. As shown in the example below, you can see if there are any outstanding pull requests and whether someone is already assigned to review them. If you come across an outstanding pull request, you have two options to move forward:

1. Leave the pull request alone, as someone else has already been assigned and will look at it.
2. Add yourself to the assignees (you can have multiple), and give a 2nd opinion on the code.

When you have a team working on a project, you can use a combination of GitHub and JIRA/Trello to increase transparency and efficiency. For example, your developers can use GitHub to keep track of pending reviews while using JIRA/Trello to provide updates for the non-technical members of the team, such as the Project Management, Delivery, and Testing teams. This process will allow everyone to easily monitor any blockers or bottlenecks and improve collaboration across the team.

Several integration tools allow synchronisation between Github and JIRA, promoting efficient workflows and better visibility.

⚠️ Problem 3: Too many tickets awaiting review

You have a reasonable number of developers on your team, say around 15-20. They're powering through work; however, the Awaiting Revew column is getting out of hand. There's a bottleneck of tickets just sitting to be reviewed, and no one is looking at them for one of many reasons, such as:

1. "I haven't had time."
2. "I'm busy doing my work."
3. "I don't have the skill set to feel comfortable approving that project area."

Solution

A Work in Progress (WIP) limit can be set for the Awaiting Review column, allowing each developer to have only one or two tickets in this column at a time. Doing so keeps the number of tickets waiting to be reviewed to a minimum, helping maintain momentum. In addition, it encourages the team to prioritise completing work and moving it forward.

⚠️ Problem 4: Too much back forth

Most commonly, PRs are rejected due to small aspects such as:

• Spelling mistakes
• Poorly named functions
• Code layout
• Differences in coding styles

As a result, the PR process is extended with unnecessary back and forth.

Solution

A coding team should collectively agree upon coding standards and styles to promote consistent and readable code. Ensuring that everyone on the team is on the same page and can easily understand each other's code.

By adhering to standards, code maintainability and scalability can be greatly improved, reducing the likelihood of errors or conflicts. Additionally, it helps with onboarding new team members and allows for easier collaboration across the team.

Implementing coding standards and styles creates a more efficient and effective coding environment. In addition, it reduces the amount of back and forth during the review process because the code author should have adhered to the coding standards previously agreed upon.

Extra things to consider

To speed up this process even further, you can install a linter. And a pre-commit script into your project. For example, there are

• Flake8 or Black - Linting for Python
• StyleCop & ReSharper - Linting for C#
• ESLint - Linting for Javascript
• StyleLint - Linting for CSS

A linter is a tool that analyses your code and checks it against a set of rules or guidelines. The goal of a linter is to catch common mistakes or issues in your code before you run it, helping you catch and fix errors early.

For example, a linter might check your code for things like unused variables, missing semicolons, or undefined function calls. By catching these issues before you push your code for a PR, you can avoid basic errors leading to Pull Request delays.

Example with ESLint and JS Project

You can utilise a tool like Husky. Husky is a tool that allows you to add Git hooks to your project. Git hooks are scripts that run automatically at certain points in the Git workflow, such as before or after a commit. By adding a Husky hook, you can run custom scripts to perform tasks like linting, formatting, or testing your code before you commit it.

Firstly, add Husky to your project.

npm install husky eslint --save-dev

Initialise Husky in your project:

npx husky-init && npm install

Open your Package.json file within a js project, and copy the following code.

"scripts": {
 "lint": "eslint .",
 "lint-staged": "lint-staged",
 "precommit": "npm run lint-staged"
},

The above code defines a new script called precommit that will run when you try to commit changes.

Add the following code to your package.json file to configure lint-staged to run ESLint on staged files.

This tells lint-staged to run ESLint with the --fix option on all JavaScript files staged for commit and then automatically add any changes to Git.

"lint-staged": {
 "*.js": [
 "eslint --fix",
 "git add"
 ]
},

Finally, commit your changes to Git and verify that the pre-commit script runs successfully by making a change to a JavaScript file and attempting to commit it. If ESLint detects any errors or warnings, the commit will be prevented until you fix them.

Conclusion

We've looked at implementing the following:

• A workflow/ticketing system with boards.
• A communication system.
• How to make it more efficient by adding coding standards and pre-commit scripts.

I hope this article has offered helpful information or things to consider to make things more efficient.

Feel free to give me a follow on Twitter and subscribe for more articles.