Best practices on code reviews and MergeTimer

Here are some general tips on how to conduct code reviews, manage pull requests, and utilize MergeTimer effectively.

Small PRs.

PRs/MRs should be small (400-800 locs), focusing on a single change and purpose at a time.

Large changes can slow down async code reviews if we aim to maintain quality, or reduce quality if we aim to maintain speed.

So, what can we do when tasks are large and require many changes that can't be divided into functional small subtasks?

  • Feature flags. Feature flags are a powerful tool in trunk-based development. They allow you to merge incomplete features into the main branch without affecting the production environment. By using feature flags, you can deactivate parts of the code that are not ready for release, ensuring that they do not interfere with the deployed application. This approach enables continuous integration and deployment, as you can safely deploy code that includes unfinished features.
    In practice, you can use feature flag management tools like LaunchDarkly, Flagsmith, or Unleash to manage your feature flags effectively.
  • Stacked pull requests. This approach involves breaking down a large task into smaller, reviewable pull requests (PRs), each containing less than 400 lines of code (loc). These smaller PRs are stacked on top of each other, meaning each subsequent PR depends on the previous one. This method allows for more manageable and focused code reviews. However, since these PRs are interdependent, changes in an earlier PR may require updates to the subsequent PRs. This can lead to additional time spent synchronizing branches through 'merge' or 'rebase'. To streamline this process, you can use tools like git-town, ghstack.
  • Pair programming. Some companies use pair programming as the default way of developing. This also means that the code is already being reviewed by 2 people while being developed, so you don't need to do a conventional async code review afterwards. It can also be combined with feature flags.

Communication in code reviews.

Normally code reviews are made in a written format. Humans communicate much worse in written format. Your corporal movements are not there. Your voice tone is not there. So misunderstandings are much more common.

This is why some new communication frameworks are raising such as conventional comments. Similar to conventional commits, but applied to code review comments. Where you explicitly specify the intention behind your comments. For example:

question (non-blocking): At this point, does it matter which thread has won?
Maybe to prevent a race condition we should keep looping until they've all won?

Where non-blocking means that this comment is not blocking the PR from being accepted.

Some other tips in this area are:

  • Ask questions instead of affirmations.
  • Be kind.
  • If you have more seniority, spend time on mentoring, this pays off exponentially.
  • If you don't agree on a specific topic with a teammate:
    • Don't take it personally.
    • Listen carefully to the other explanation.
    • Discuss it in person or in a call, where effective communication is easier.

Configure branch protection rules.

To ensure code quality and stability, configure branch protection rules to block merges if the continuous integration (CI) tests are not passing. This ensures that no code can be merged into the main branch unless all tests have successfully passed. By enforcing this rule, MergeTimer will also be unable to merge any pull requests that do not meet the CI requirements, maintaining the integrity of the codebase. Most version control platforms, like GitHub, GitLab, and Bitbucket, offer settings to enforce these rules. For example, in GitHub, you can navigate to the repository settings, select "Branches," and then configure the protection rules to require status checks to pass before merging.

Reply fast to code review messages, but don't break your flow state.

Being "in the flow state" means you are super focused on a task: either designing, programming or doing anything else. Some DevEx frameworks (such as the one presented in the paper: Devex: What Actually Drives Productivity) place the flow state as a top priority subject for developer productivity.

Flow state is very important for the efficiency of the group, so if you are a developer and you are in flow state, try to keep yourself in that state as long as you can, avoid switching tasks and interruptions.

But if not, reply fast to code review messages!

Check out PR Slack channels for faster code review conversations.