Why Pull-Requests or Merge-Requests often fail
I have been working in various development teams for over 10 years and we often work with pull requests and merge requests.
A pull request or merge request is a way for a developer to propose changes to a codebase and ask for them to be reviewed asynchronously and merged into a protected branch such as the 'Develop" or "Main" branch.
In my experience, the PR technique often reduces developer productivity.
I would like to present you with real (at least experienced) scenarios where pull requests and merge requests are not effective and show you some alternative techniques.
❌ Irrelevant Pull-Requests
I often experience that we create PRs for the smallest code changes, e.g. to bring a sealed secret into a main (one line of code, nobody can check the correctness of the value). This is only a small configuration change.
Such changes should not lead to PR. For such changes, we should trust the developer who makes the change. And if not, then at least do pair programming.
❌ Pull-Requests that are too big
When a developer works on a feature for more than a week, he or she creates a bunch of new code and code changes. When he or she creates a PR, this PR is so large that it's a big challenge to review such a large PR.
For reviewing such large pieces, a synchronous code walkthrough is imho much better (more effective and efficient) than taking the whole development team - each on their own - to this code review.
❌ Epic asynchronous technical detail discussions
Who hasn't already experienced it: A PR leads to an epic technical detail discussion and that too asynchronously via comment & reply in the review tool.
And why?
IMHO, PR technique encourages this kind of discussion and communication. And it's even worse when the developers are remote.
❌ The blocked developer
If you create a PR as a developer, you will be blocked. You have to wait for your colleagues to approve your code at least once (and hopefully not with the "changes required" flag).
❌ Multiple work items in progress
If you are blocked on a work item due to an open PR, you must move on to the next work item. This increases your WIPs. This reduces your productivity because you have to make many context switches.
❌ A lot of context changes
Not only the PR creator experiences a context change, but also his colleagues who receive the pull request or the merge request. Context changes have hardly any impact on developer productivity.
Alternative techniques
✅ Rethink the need for a code-review
Discuss the goal of a code review (and pull requests or merge requests) in the development team:
- Does it really make sense to have every code change reviewed via pull-request by every developer on the team?
- What do we want to achieve with this?
✅ Synchronous code walk-through
When someone develops a big feature over several days. I really like synchronous code walkthroughs, preferably during the development of the feature. It doesn't have to be complete. It's really helpful for spreading knowledge to understand the ideas behind the implementation, and sometimes it can give a good technical discussion.
✅ Pair-Programming
Why do we create PRs for asynchronous code reviews when we could do pair programming as a team. This is much more effective for reviewing, discussing and sharing knowledge than a PR.
✅ Trunk-Based Development
Trunk-based development (TBD) is a software development method that focuses on working on a single, shared code base, often referred to as a "trunk" or "main" branch".
Developers frequently push their changes to the trunk, and automated testing is performed regularly to ensure that the code remains stable.
Trunk-based development encourages collaboration between developers as they are all working on the same codebase.