Pull requests (PR) are part of the developer workflow within Github. You may hear this term as merge requests in Gitlab or other tools. The workflow for developers on my project is managed within Github. This is where user stories are listed for the sprint as issues and developers assign themselves the issues during the sprint.
work·flow noun the sequence of industrial, administrative, or other processes through which a piece of work passes from initiation to completion.
Developers work based off the requirements within the issue and then make a PR when the work is completed. While the developer waits for a code review on that PR they will typically assign themselves another issue to go work. Now the developer has two issues - one in code review and another in-progress. Let's say the developer finishes the 2nd issue and makes another PR, they now have 2 PRs in code review.
This is where the 2 PR rule comes in to play. Once the developer reaches this point, they should not pick another task until one of their current PRs is merged into the baseline. This rule is mainly to encourage the developers and the team to review code, provide feedback and get the code merged. While the developer isn't supposed to pick up another task that doesn't mean they shouldn't be working. During this time the developer should ask other developers for a code review. The code review will typically result in comments and requested changes or get approval to merge the code. If there are requested changes the developer is able to immediately begin addressing those comments and update their PR using this rule. If their code is merged then the developer is free to assign themselves another task to work on.
There will be times where no other developers are available to do a code review because they are focused on their own task, bug, or fire. In those cases the developer waiting for code reviews on their 2 PRs should seek out another developer who may need help and pair-program. They say two minds are better than one and pair-programming can help a struggling develop get over a technical hump with another set of eyes. By pair-programming the idea is that the work will result in better quality code and merged in to the baseline faster.
I have seen problems arise when a rule like this wasn't in place. Mainly less code is delivered by the developer because their is no focus to get code merged or address comments in a timely fashion. To illustrate, take for example a developer who has 4 PRs and assigned themselves another task to work. They now have 5 tasks in a state of code review or work-in-progress. As code reviews for the 4 PRs happen and comments are provided, the developer is focused on their 5th task and works to get a PR up for that one. When they eventually get around to addressing comments they may now also have file conflicts they need to resolve that resulted from the time it took to address those comments. Once they address those comments it may take some time to get another code review on those PRs. So this developer now has 5 PRs and nothing has been delivered to the baseline. If this process continues, then the developer begins work on a new task which escalates the problem. Eventually the developer is the bottleneck to get many tasks completed.
I summarized the pro and cons from my experience of using this approach.
- Focus on delivering code
- Encourage developers to do code reviews (everybody participates)
- Help other developers who may be struggling through pair-programming
- Potentially delay other work from starting
- Can be frustrating for developers waiting on code reviews
I'm interested in hearing your thoughts!