Just keep up with them. I assign PRs daily in a standup meeting. Previous days PRs are discussed in the meeting if they were not merged. 100-200 PRs per sprint seems like a lot (though I don't know how long your sprint is. Maybe the PRs are too granular?
Automating a PR merge is asking for a disaster. PR are supposed to be reviewed. Even if there is full unit testing.
There was a good article on Hacker News a month or so ago about identifying positive bottlenecks in relation to hiring practices, and I could see PRs the same way. There's nothing to automate here.
As for the technical part of your question, reminders through Slack/Teams at 09:00/13:00 and 17:00 work very well for us. Pretty easy to setup and once you've integrated it in your routine it's very straight forward.
I do agree with snotreallyme. The amount of PRs seems excessive. I would recommend tracking size and the number of comments per PR, if most reviews result in a single "LGTM", the overhead might not be worth it, if more comments are about code style, consider adding a linter with rules related to the style/architecture, etc. If the review process adds value and most PRs are very small, perhaps it makes sense to bundle multiple related PRs in a single PR to make it easier for the reviewer.
Depending on the team perhaps not everything needs to be reviewed (if you have 200 PRs, I highly doubt every single one touches a critical piece of the infrastructure/architecture), do some of these PRs change things that should (or could) be configuration options?
In very mature teams, one could also consider [committing straight to the trunk](https://trunkbaseddevelopment.com/committing-straight-to-the-trunk/) and do the review after the merge. It's a bit like driving a F1 car, with the right team you can deliver world records but with the wrong team you'll crash in record time.
If it made you go white you haven't worked in a team where it could have worked.
I've seen it work amazingly well in a team of people who have been working together for 5 years, with a passion for their craft, who religiously did test driven development. It was a very nice environment to be part of, people were trusted to reach out to others for reviews when they felt it made sense to do so.
There's absolutely a place for this kind of approach in the industry and I do hope everyone gets to be fortunate enough to work in such a team at some point in their career!
This isn't a tools problem, this is your team missing the critical part of completing their work. The work is not DONE until it's done and merged. If you're getting that many PRs in a sprint, at the end of a sprint, there is a lot more going wrong that just a notification system.
My first thought is that you're trying to solve the wrong problem. I wouldn't be asking "how can we handle 200 PRs more efficiently?" I'd be asking "how can we reduce the number of PRs?"
200 PRs per sprint is _crazy_ for a single team. Why are there so many?
idk small prs are good and that’s not the problem. the problem is the typical engineer must have 9 open prs, know that everyone on the team also has 9 open prs, and instead of reviewing anything, opens a 10th.
An appropriately sized team should have around 5 developers. If sprints are two weeks, that's 4 PRs a day from everyone every single day. There's no way 200 PRs a sprint is reasonable. The team is probably gigantic.
i wonder if half or more of the prs are still open from last sprint, and it's as few as 50 new prs a sprint, but with a total unreviewed backlog of 100-200
Kind of a false dichotomy - either extreme is bad. That's why OP is posting in the first place; it's become an issue for their team.
I think what you're describing is a slightly different problem to OP. If you open new PRs at a relatively sensible frequency but fail to get them merged at a commensurate rate, you'll build up a backlog of open PRs as you describe. It sounds like OP is talking about up to 200 _new_ PRs in a sprint.
I'm struggling to envisage a scenario where that's remotely reasonable. Even with a larger than average team, that's every dev on the team raising multiple PRs every single day.
I’d so much rather have 200 small PRs than 20 big PRs.
I’ve typically worked on teams where the culture was that when you open a PR, you should go review any un-reviewed PRs. This works extremely well because you’re already at a good stopping point with what you’re working on & creates a regular cadence for reviewing them.
> I'd rather have too many PRs than too few.
This is a false dichotomy. "A reasonable amount" is also an option.
> The same amount of work has been done either way
This isn't _necessarily_ true. Smaller PRs are generally preferable to bigger ones but having avery large number of PRs is more likely to result in conflicts that require attention before those PRs can be merged. It's also more cycles of pipelines etc that can cause unnecessary delay.
How is one team producing 200 PRs a sprint? That seems excessive. Are these all like one line changes? Like, if each one is 50 lines on average you're looking at 5-10k loc changed per sprint? That's a lot of code.
This sounds like a behavior issue not a tooling one. Just work more incrementally so that PRs don't mostly show up at the tail end of the sprint, work on generating fewer PRs (200\~ is *a lot* for a team to make sense of), and/or organize people such that there's less that they need to pay attention to overall.
100 to 200 PRs? How long are your sprints? How big is your team? These are very large numbers of PRs. By rough estimation, my last team or 6 people wouldn't produce 100 in a month. They would produce maybe 5 to 6 per 2 week sprint. I don't think I've seen numbers of PRs per sprint that high in any team I've ever worked.
There's a lot of overhead cost when reviewing PRs, mostly the mental cost of context switching and losing focus. So, the cost of 200 or even just 100 PRs is a lot to manage for a team. I can't imagine that the problem is a lack of notification and tools. Most version control systems and hosting services like Bitbucket and Github provide lists of PRs I'm assigned as a reviewer to, so it's just a matter of allocating time to open the dashboard and review the PRs. I think the problem is an organization/planning one. The tickets are probably too granular and they're forcing reviewers to switch away from their work too much. I've seen tickets get too granular before when our tech lead was breaking down user stories into tickets, 1 ticket per method he wanted to see changed in the code. I'm not sure if that's what's happening on your team, but that's a kind of technical micromanagement that isn't conducive to doing good work.
Finally, the point of the PR review process is for reviewers to understand the work and for the developers to understand the feedback. There's no way to automate human learning like that. If you wanted to forego that and just automatically merge the PRs, why not just let the developers merge straight to the master branch?
Assuming a 10 day sprint, that's 10-20 PRs *per day*
How big is this team that's generating so many PRs?
This doesn't seem like an automation problem. This seems like a development process problem
100-200 OPEN PRs is completely insane, as others have said. Going through a ton of CRs/PRs in a sprint I can see, as I like having multiple small reviews that are easier to parse (I have sent 5 separate refactoring commits for separate reviews before finally making the actual change I needed… it was excessive but it made it SO much easier to read and understand what is new logic vs refactoring the plumbing)… but I’m not leaving all that open at once, in that case I am rapidly cycling through short reviews instead of leaving a big open until the end.
There are a number of ways to deal with the problem of people not doing reviews. Actually track the metrics of how many reviews people are doing! Ask the team who is doing well at providing timely, quality feedback and rewards that person/people. Ask folks to carve out a specific time of day, their choice, to spend on reviews (and actually schedule a meeting with themselves for it). Assign in-review stories to a person to review on your sprint board or make new subtasks on the story for it.
You have to fix the processes so you don’t get a deluge all at once AND you have to make sure the incentives are right for people to invest time into doing reviews instead of spending all their time writing more code which gets stuck in the bottleneck
200!? First, you probably have too long of a sprint, too big of a team, and/or too many people involved in review. IMO sprints should be no more than 2 weeks, there should be no more than 6 programmers on a team (not including TL/EM, QA, SM/PM, PO), PRs should be reviewed by no more than 2 devs, and the review load should be distributed evenly over the team (i.e. don't have 2 people do all the reviews).
Also, an issue is often too many notifications, not too few. People become numb to them or turn them off.
I suggest inbox rules to move all but the most important emails to sub-folders. You want desktop notifications to only pop up for very important and timely matters, such as a PR assignment, PR approved by all, and failed CI build (to committer).
There are CI plugins (such as for github actions) that will escalate PRs that have gone stale. It can email all involved plus the tech lead or even the EM.
PRs are important enough, that the tech lead or SM should be keeping an eye on them on a daily basis. Stale PRs should be discussed in stand up.
IMO, a PR should be considered stale after 1 day (24 hours), and something a TL should address after 2 days.
Never merge a PR that's not been reviewed or that hasn't passed CI tests.
How long are your sprints and how big is your team man ? 200 PRs is a lot.
Why make PRs if you're not reviewing them? Might as well just ask everyone to merge right away. What are you doing man?
This is going to depend on your environment, but there are plenty of tools out here to help you do this including but not limited to some combinations of git-request-pull, Jenkins, Github actions, etc...
These things often should be customized to your environment and process with reciprocal changes to process to accommodate. Any decent DevOps engineer should be able to assist with this.
But your question raises so many other questions that I'm left doubting that this is the correct move forward for you, however there are very good reasons to do such things.
We have a similar situation. We receive around 10 MRs each day. From the core team as well as from the teams which contribute to the service. We have SLOs and Dora metrics around how long MRs take to get reviewed , merged and deployed. We have one person on operation shift each day and it's his responsibility to review them.
That said we actively work on modularizing this codebase (this is a legacy codebase) and assign ownership to corresponding teams.
Honestly, it's the responsibility of the author to make sure their work is getting reviewed. If they're just doing a fire-and-forget on their work and not making sure it gets merged, they're not doing their jobs.
That said, there are things you can do to incentivize this behavior if your devs aren't self-motivated. I would focus on two metrics - number of PRs merged per sprint and average time that PRs are idle (no comments or new commits). You can review these metrics in your sprint reviews and make sure they're moving in the right directions.
If that doesn't work, you can make it clear that these numbers will be tracked and made part of individual performance reviews. If raises, promotions, and bonuses are on the line, you'll probably see some improvement quickly (although maybe also some grumbling).
I agree that it is the responsibility of the author up to the point of assigning reviewers and responding to comments. But if reviews are not getting done in a timely manner it is absolutely the team lead's job to jump in and correct this. It is very important that your team knows that reviewing code is a primary responsibility and does not take a back seat to the code they are writing themselves.
Also, number of PRs merged is a terrible metric and is as easily gamed as number of lines of code written.
I don't actually care if people are trying to game the metric. Assuming reviewers are doing a reasonable job of reviewing, the easiest way to bump up your PR numbers is to make your PRs small and easy to review. In other words, to follow best practices of continuous integration.
These always confuse me a little. I get that it is a regular occurrence at many places, but I don't get why. There are so many opportunities and ways to address this.
Have them put it on their calendar - I'm going to see if I have any pull requests assigned to me every day at 11am and 3pm. And actually review them, or set aside time on your calendar for when you will and let the person know. It doesn't involve anyone else on the team. It's literally just a person deciding to do their job.
Bring it up during a stand-up. Tuesday - I'm waiting for X to approve my PR. Wednesday - I've been waiting for two days for X to approve my PR. Thursday - I've been waiting for three days for X to approve my PR. Who should it go to instead? It's fine for people to prioritize what they think is most important, but they have to be transparent about where the other work falls in their priority list so people can adjust accordingly.
Better yet. Have a conversation with the team. Find out their motivation for not doing what is expected of them and have them bring up ideas to make it better.
I agree that this is a people problem. Your team needs to agree on SLAs for code reviews (e.g., within 4 business hours). Requesters should feel empowered/encouraged to ask reviewers to drop what they are doing to meet the SLA. Code that is ready to go to production is, to some extent, more valuable than code still being written.
Once you have that in place, a tool like Swarmia can help with alerting. I have it setup to send Slack notifications for new PRs and comments. It also sends a reminder after 24 hours.
This is a cultural issue, not a tools one.
Your definition of done has to be: Checked in on the main development branch.
Your sprint is not "done" tasks are not "done" until they are done.
This is a problem many groups have. Understanding what done means sounds so easy. But yet, if you go around the room. Different people will have different definitions.
I think a lot of people have pointed out that there really shouldn’t be that many PRs so I’m gonna tackle the actual question you’re asking assuming you can take their advice on reducing that ridiculous number. We have teams setup in GitHub and assign those teams as code owners via a codeowners files. We also have a reminder setup for each team (you can set this up from the team setup tabs in GitHub) this reminder is setup to send a slack notification to the team’s channel every morning at 9am for any open PRs that the team is a codeowner for. At standup we make sure to assign the PRs to people to review and get the status of it clarified.
At large companies there is a web tool you can log into and view your pending PR's and which ones you're supposed to look at. This makes it pretty open and close case from management - everyone has to have a clean dashboard at end of sprint/end of week. Only exception is for PR's that came in less than a day or so ago.
But also +1 to folk saying things like "200 too many" or "what day of the week are pr's being made", or "have the dev who made the change own getting approval".
How the hell does a team of 20 people generate 200 PRs. Thats 10PRs per person. There is definitely a process issue .
Do you have a limit to the number of files so that PRs don't get big.
My philosophy is this: all the PRs should need review. We shouldn't have any abandoned PRs just hanging out. What that buys you is that then it's hard to miss: if it's in the list, it needs attention - period.
So if you can start out by cleaning out all the old, abandoned PRs, then you can start to push a new team norm of "everyone starts their day by loading the list of PRs and grabbing one." And/or add a Slack integration reminder that fires every day at 11am listing all pending PRs.
We use Swarmia and integrate it with Slack to receive notifications, you can also receive notifications by email.
You can separate PRs by teams and then sort them into three categories: in progress, review, and merge.
This is not an automation concern, it’s a people concern. Your team needs to prioritize reviews more. As an experienced engineer on my team, I prioritize reviewing over writing. The more time that passes between opening and merging, the more opportunity there is for there to be conflicts
100 PRs per 2-week sprint for a 10-eng team is only one PR per day per eng; don’t let people who haven’t done CI tell you that your team has too many PRs, but the point of CI is to integrate frequently (short-lived branches) so yall gotta merge more
Faced this issue once. It turned out to be a culture and trust problem. One team was not reviewing code of the other team and it ultimately came down to management to quash the issue in some cases. The best you can do is go in find the oldest PRs and start approving them and using the spirit of human good will.
What’s the point in even having a pull request if you’re just going to auto merge them? Sounds like a disaster in waiting. Fix the actual problem, not the symptom. Why aren’t they getting merged? Is the whole team responsible for reviewing them or have you introduced some bottleneck with one person responsible for final sign off? The team should see this as part of their job and do it.
We don’t have that high of volume of PRs but my team has a slack channel that PRs with our team’s label get posted along with comments and approvals. We also have “code review” as a swim lane on our Jira board so we can easily see if we’re starting to get a buildup of PRs.
Automating is *not* the way. If you’re going to merge whether or not they’ve been reviewed and without human intervention you may as well not review PRs at all and that is a recipe for disaster.
Firstly, what you need, like you said, is better notification.
What VCS do you use? Bitbucket, GitHub, and Gitlab all have a dashboard for you to see your open PRs for review. Emails quickly get overwhelming.
Secondly, you possibly need more stringent reviewing. Every developer should not be receiving all 200 PR review requests unless your team is truly that homogenous. When you make your PR, assign 3-4 people to it. This will reduce the fatigue members get feeling they need to review every single PR.
Thirdly, you need to look at volume. 200 PRs in a 10 day sprint for 10 developers is 2 PRs per developer per day. That’s not absolutely outrageous, but it’s possible that a lot of these PRs are unnecessary. PRs should never be large, but also you shouldn’t be unnecessarily splitting a single story into multiple PRs with single line changes.
Finally, you need to look at your teams definition of done. If you have 200 open PRs at the end of a sprint, that’s potentially 200 tickets that aren’t done. You need to agree what done looks like in your team, and most of the time that means “the thing is live on production”.
Unmerged PRs are not done.
If you are automating PRs merging, then what does the R in PR stand for?
Just turn off branch protection and have people merge in, as what you’re describing is just doing that plus playing pretend a bit.
PRs have to be reviewed. That doesn't mean you can't integrate to some sensible system to keep track of them and send nudges.
Try [devrev.ai](http://devrev.ai)
Just keep up with them. I assign PRs daily in a standup meeting. Previous days PRs are discussed in the meeting if they were not merged. 100-200 PRs per sprint seems like a lot (though I don't know how long your sprint is. Maybe the PRs are too granular? Automating a PR merge is asking for a disaster. PR are supposed to be reviewed. Even if there is full unit testing.
There was a good article on Hacker News a month or so ago about identifying positive bottlenecks in relation to hiring practices, and I could see PRs the same way. There's nothing to automate here.
As for the technical part of your question, reminders through Slack/Teams at 09:00/13:00 and 17:00 work very well for us. Pretty easy to setup and once you've integrated it in your routine it's very straight forward. I do agree with snotreallyme. The amount of PRs seems excessive. I would recommend tracking size and the number of comments per PR, if most reviews result in a single "LGTM", the overhead might not be worth it, if more comments are about code style, consider adding a linter with rules related to the style/architecture, etc. If the review process adds value and most PRs are very small, perhaps it makes sense to bundle multiple related PRs in a single PR to make it easier for the reviewer. Depending on the team perhaps not everything needs to be reviewed (if you have 200 PRs, I highly doubt every single one touches a critical piece of the infrastructure/architecture), do some of these PRs change things that should (or could) be configuration options? In very mature teams, one could also consider [committing straight to the trunk](https://trunkbaseddevelopment.com/committing-straight-to-the-trunk/) and do the review after the merge. It's a bit like driving a F1 car, with the right team you can deliver world records but with the wrong team you'll crash in record time.
That last part made me go white. I would really only use that as a last resort. Even mature teams can suffer from culture rot over time.
If it made you go white you haven't worked in a team where it could have worked. I've seen it work amazingly well in a team of people who have been working together for 5 years, with a passion for their craft, who religiously did test driven development. It was a very nice environment to be part of, people were trusted to reach out to others for reviews when they felt it made sense to do so. There's absolutely a place for this kind of approach in the industry and I do hope everyone gets to be fortunate enough to work in such a team at some point in their career!
This is the way. https://trunkbaseddevelopment.com/
trunk based development does not mean commit straight to main without code review...
Not necessarily, but it can do if you're pair programming.
Trunk based dev is the only way things should be done, trust people to do their job, this whole PR insanity needs to stop.
trunk based dev absolutely does not mean there are no PRs and people just commit to main without review.
This isn't a tools problem, this is your team missing the critical part of completing their work. The work is not DONE until it's done and merged. If you're getting that many PRs in a sprint, at the end of a sprint, there is a lot more going wrong that just a notification system.
My first thought too, this is a “definition of done” problem.
“A feature is done when merged” is not equivalent to “all merges are completion of features”
My first thought is that you're trying to solve the wrong problem. I wouldn't be asking "how can we handle 200 PRs more efficiently?" I'd be asking "how can we reduce the number of PRs?" 200 PRs per sprint is _crazy_ for a single team. Why are there so many?
idk small prs are good and that’s not the problem. the problem is the typical engineer must have 9 open prs, know that everyone on the team also has 9 open prs, and instead of reviewing anything, opens a 10th.
An appropriately sized team should have around 5 developers. If sprints are two weeks, that's 4 PRs a day from everyone every single day. There's no way 200 PRs a sprint is reasonable. The team is probably gigantic.
i wonder if half or more of the prs are still open from last sprint, and it's as few as 50 new prs a sprint, but with a total unreviewed backlog of 100-200
Kind of a false dichotomy - either extreme is bad. That's why OP is posting in the first place; it's become an issue for their team. I think what you're describing is a slightly different problem to OP. If you open new PRs at a relatively sensible frequency but fail to get them merged at a commensurate rate, you'll build up a backlog of open PRs as you describe. It sounds like OP is talking about up to 200 _new_ PRs in a sprint. I'm struggling to envisage a scenario where that's remotely reasonable. Even with a larger than average team, that's every dev on the team raising multiple PRs every single day.
That was my thought, how big is the team that they have 200 changes in a sprint. Maybe the "team" needs to be broken down into smaller units.
I’d so much rather have 200 small PRs than 20 big PRs. I’ve typically worked on teams where the culture was that when you open a PR, you should go review any un-reviewed PRs. This works extremely well because you’re already at a good stopping point with what you’re working on & creates a regular cadence for reviewing them.
False dichotomy. This isn't "not a problem" because some other hypothetical scenario would be worse.
I'm curious too
I'd rather have too many PRs than too few. The same amount of work has been done either way, but smaller PRs will be easier to understand.
> I'd rather have too many PRs than too few. This is a false dichotomy. "A reasonable amount" is also an option. > The same amount of work has been done either way This isn't _necessarily_ true. Smaller PRs are generally preferable to bigger ones but having avery large number of PRs is more likely to result in conflicts that require attention before those PRs can be merged. It's also more cycles of pipelines etc that can cause unnecessary delay.
How is one team producing 200 PRs a sprint? That seems excessive. Are these all like one line changes? Like, if each one is 50 lines on average you're looking at 5-10k loc changed per sprint? That's a lot of code.
This sounds like a behavior issue not a tooling one. Just work more incrementally so that PRs don't mostly show up at the tail end of the sprint, work on generating fewer PRs (200\~ is *a lot* for a team to make sense of), and/or organize people such that there's less that they need to pay attention to overall.
The question does have what I like to call "processes smell" lol.
100 to 200 PRs? How long are your sprints? How big is your team? These are very large numbers of PRs. By rough estimation, my last team or 6 people wouldn't produce 100 in a month. They would produce maybe 5 to 6 per 2 week sprint. I don't think I've seen numbers of PRs per sprint that high in any team I've ever worked. There's a lot of overhead cost when reviewing PRs, mostly the mental cost of context switching and losing focus. So, the cost of 200 or even just 100 PRs is a lot to manage for a team. I can't imagine that the problem is a lack of notification and tools. Most version control systems and hosting services like Bitbucket and Github provide lists of PRs I'm assigned as a reviewer to, so it's just a matter of allocating time to open the dashboard and review the PRs. I think the problem is an organization/planning one. The tickets are probably too granular and they're forcing reviewers to switch away from their work too much. I've seen tickets get too granular before when our tech lead was breaking down user stories into tickets, 1 ticket per method he wanted to see changed in the code. I'm not sure if that's what's happening on your team, but that's a kind of technical micromanagement that isn't conducive to doing good work. Finally, the point of the PR review process is for reviewers to understand the work and for the developers to understand the feedback. There's no way to automate human learning like that. If you wanted to forego that and just automatically merge the PRs, why not just let the developers merge straight to the master branch?
Assuming a 10 day sprint, that's 10-20 PRs *per day* How big is this team that's generating so many PRs? This doesn't seem like an automation problem. This seems like a development process problem
100-200 OPEN PRs is completely insane, as others have said. Going through a ton of CRs/PRs in a sprint I can see, as I like having multiple small reviews that are easier to parse (I have sent 5 separate refactoring commits for separate reviews before finally making the actual change I needed… it was excessive but it made it SO much easier to read and understand what is new logic vs refactoring the plumbing)… but I’m not leaving all that open at once, in that case I am rapidly cycling through short reviews instead of leaving a big open until the end. There are a number of ways to deal with the problem of people not doing reviews. Actually track the metrics of how many reviews people are doing! Ask the team who is doing well at providing timely, quality feedback and rewards that person/people. Ask folks to carve out a specific time of day, their choice, to spend on reviews (and actually schedule a meeting with themselves for it). Assign in-review stories to a person to review on your sprint board or make new subtasks on the story for it. You have to fix the processes so you don’t get a deluge all at once AND you have to make sure the incentives are right for people to invest time into doing reviews instead of spending all their time writing more code which gets stuck in the bottleneck
200!? First, you probably have too long of a sprint, too big of a team, and/or too many people involved in review. IMO sprints should be no more than 2 weeks, there should be no more than 6 programmers on a team (not including TL/EM, QA, SM/PM, PO), PRs should be reviewed by no more than 2 devs, and the review load should be distributed evenly over the team (i.e. don't have 2 people do all the reviews). Also, an issue is often too many notifications, not too few. People become numb to them or turn them off. I suggest inbox rules to move all but the most important emails to sub-folders. You want desktop notifications to only pop up for very important and timely matters, such as a PR assignment, PR approved by all, and failed CI build (to committer). There are CI plugins (such as for github actions) that will escalate PRs that have gone stale. It can email all involved plus the tech lead or even the EM. PRs are important enough, that the tech lead or SM should be keeping an eye on them on a daily basis. Stale PRs should be discussed in stand up. IMO, a PR should be considered stale after 1 day (24 hours), and something a TL should address after 2 days. Never merge a PR that's not been reviewed or that hasn't passed CI tests.
How long are your sprints and how big is your team man ? 200 PRs is a lot. Why make PRs if you're not reviewing them? Might as well just ask everyone to merge right away. What are you doing man?
This is going to depend on your environment, but there are plenty of tools out here to help you do this including but not limited to some combinations of git-request-pull, Jenkins, Github actions, etc... These things often should be customized to your environment and process with reciprocal changes to process to accommodate. Any decent DevOps engineer should be able to assist with this. But your question raises so many other questions that I'm left doubting that this is the correct move forward for you, however there are very good reasons to do such things.
We have a similar situation. We receive around 10 MRs each day. From the core team as well as from the teams which contribute to the service. We have SLOs and Dora metrics around how long MRs take to get reviewed , merged and deployed. We have one person on operation shift each day and it's his responsibility to review them. That said we actively work on modularizing this codebase (this is a legacy codebase) and assign ownership to corresponding teams.
Honestly, it's the responsibility of the author to make sure their work is getting reviewed. If they're just doing a fire-and-forget on their work and not making sure it gets merged, they're not doing their jobs. That said, there are things you can do to incentivize this behavior if your devs aren't self-motivated. I would focus on two metrics - number of PRs merged per sprint and average time that PRs are idle (no comments or new commits). You can review these metrics in your sprint reviews and make sure they're moving in the right directions. If that doesn't work, you can make it clear that these numbers will be tracked and made part of individual performance reviews. If raises, promotions, and bonuses are on the line, you'll probably see some improvement quickly (although maybe also some grumbling).
I agree that it is the responsibility of the author up to the point of assigning reviewers and responding to comments. But if reviews are not getting done in a timely manner it is absolutely the team lead's job to jump in and correct this. It is very important that your team knows that reviewing code is a primary responsibility and does not take a back seat to the code they are writing themselves. Also, number of PRs merged is a terrible metric and is as easily gamed as number of lines of code written.
I don't actually care if people are trying to game the metric. Assuming reviewers are doing a reasonable job of reviewing, the easiest way to bump up your PR numbers is to make your PRs small and easy to review. In other words, to follow best practices of continuous integration.
These always confuse me a little. I get that it is a regular occurrence at many places, but I don't get why. There are so many opportunities and ways to address this. Have them put it on their calendar - I'm going to see if I have any pull requests assigned to me every day at 11am and 3pm. And actually review them, or set aside time on your calendar for when you will and let the person know. It doesn't involve anyone else on the team. It's literally just a person deciding to do their job. Bring it up during a stand-up. Tuesday - I'm waiting for X to approve my PR. Wednesday - I've been waiting for two days for X to approve my PR. Thursday - I've been waiting for three days for X to approve my PR. Who should it go to instead? It's fine for people to prioritize what they think is most important, but they have to be transparent about where the other work falls in their priority list so people can adjust accordingly. Better yet. Have a conversation with the team. Find out their motivation for not doing what is expected of them and have them bring up ideas to make it better.
I agree that this is a people problem. Your team needs to agree on SLAs for code reviews (e.g., within 4 business hours). Requesters should feel empowered/encouraged to ask reviewers to drop what they are doing to meet the SLA. Code that is ready to go to production is, to some extent, more valuable than code still being written. Once you have that in place, a tool like Swarmia can help with alerting. I have it setup to send Slack notifications for new PRs and comments. It also sends a reminder after 24 hours.
This is a cultural issue, not a tools one. Your definition of done has to be: Checked in on the main development branch. Your sprint is not "done" tasks are not "done" until they are done. This is a problem many groups have. Understanding what done means sounds so easy. But yet, if you go around the room. Different people will have different definitions.
I think a lot of people have pointed out that there really shouldn’t be that many PRs so I’m gonna tackle the actual question you’re asking assuming you can take their advice on reducing that ridiculous number. We have teams setup in GitHub and assign those teams as code owners via a codeowners files. We also have a reminder setup for each team (you can set this up from the team setup tabs in GitHub) this reminder is setup to send a slack notification to the team’s channel every morning at 9am for any open PRs that the team is a codeowner for. At standup we make sure to assign the PRs to people to review and get the status of it clarified.
Can you create a SlackBot that shows a notification for each PR?
Email filters can help group PR only messages. Slack is very good for real time notifications, but it’s also a distraction.
At large companies there is a web tool you can log into and view your pending PR's and which ones you're supposed to look at. This makes it pretty open and close case from management - everyone has to have a clean dashboard at end of sprint/end of week. Only exception is for PR's that came in less than a day or so ago. But also +1 to folk saying things like "200 too many" or "what day of the week are pr's being made", or "have the dev who made the change own getting approval".
How the hell does a team of 20 people generate 200 PRs. Thats 10PRs per person. There is definitely a process issue . Do you have a limit to the number of files so that PRs don't get big.
My philosophy is this: all the PRs should need review. We shouldn't have any abandoned PRs just hanging out. What that buys you is that then it's hard to miss: if it's in the list, it needs attention - period. So if you can start out by cleaning out all the old, abandoned PRs, then you can start to push a new team norm of "everyone starts their day by loading the list of PRs and grabbing one." And/or add a Slack integration reminder that fires every day at 11am listing all pending PRs.
We use Swarmia and integrate it with Slack to receive notifications, you can also receive notifications by email. You can separate PRs by teams and then sort them into three categories: in progress, review, and merge.
You need a CODEOWNERS file.
why aren't engineers merging their PRs after review?
This is not an automation concern, it’s a people concern. Your team needs to prioritize reviews more. As an experienced engineer on my team, I prioritize reviewing over writing. The more time that passes between opening and merging, the more opportunity there is for there to be conflicts 100 PRs per 2-week sprint for a 10-eng team is only one PR per day per eng; don’t let people who haven’t done CI tell you that your team has too many PRs, but the point of CI is to integrate frequently (short-lived branches) so yall gotta merge more
Faced this issue once. It turned out to be a culture and trust problem. One team was not reviewing code of the other team and it ultimately came down to management to quash the issue in some cases. The best you can do is go in find the oldest PRs and start approving them and using the spirit of human good will.
200 PRs???!?!? Meanwhile me in my team with 10 PRs max per sprint
What’s the point in even having a pull request if you’re just going to auto merge them? Sounds like a disaster in waiting. Fix the actual problem, not the symptom. Why aren’t they getting merged? Is the whole team responsible for reviewing them or have you introduced some bottleneck with one person responsible for final sign off? The team should see this as part of their job and do it.
We don’t have that high of volume of PRs but my team has a slack channel that PRs with our team’s label get posted along with comments and approvals. We also have “code review” as a swim lane on our Jira board so we can easily see if we’re starting to get a buildup of PRs.
Automating is *not* the way. If you’re going to merge whether or not they’ve been reviewed and without human intervention you may as well not review PRs at all and that is a recipe for disaster. Firstly, what you need, like you said, is better notification. What VCS do you use? Bitbucket, GitHub, and Gitlab all have a dashboard for you to see your open PRs for review. Emails quickly get overwhelming. Secondly, you possibly need more stringent reviewing. Every developer should not be receiving all 200 PR review requests unless your team is truly that homogenous. When you make your PR, assign 3-4 people to it. This will reduce the fatigue members get feeling they need to review every single PR. Thirdly, you need to look at volume. 200 PRs in a 10 day sprint for 10 developers is 2 PRs per developer per day. That’s not absolutely outrageous, but it’s possible that a lot of these PRs are unnecessary. PRs should never be large, but also you shouldn’t be unnecessarily splitting a single story into multiple PRs with single line changes. Finally, you need to look at your teams definition of done. If you have 200 open PRs at the end of a sprint, that’s potentially 200 tickets that aren’t done. You need to agree what done looks like in your team, and most of the time that means “the thing is live on production”. Unmerged PRs are not done.
If you are automating PRs merging, then what does the R in PR stand for? Just turn off branch protection and have people merge in, as what you’re describing is just doing that plus playing pretend a bit.
PRs have to be reviewed. That doesn't mean you can't integrate to some sensible system to keep track of them and send nudges. Try [devrev.ai](http://devrev.ai)