DEV Community

Discussion on: How do you incentivize developers to review Pull Requests?

Collapse
 
simonhaisz profile image
simonhaisz

Funnily enough I recently wrote a pull request guide for my team about the best practices for a good PR, both for the creator and the reviewers. Though that focus was on increasing the quality of the PRs (we'd started having too much rubber-stamp behavior with PRs) than the issue with people not reviewing that you are experiencing.

I've had prior experience with the 'people not reviewing their PRs' problem, and there are different root causes. Like @samuraiseoul said you should ask the team to find out what they say is the reason before you try to fix it.

The most common cause is the priority problem, where people prioritize working on their stuff rather than reviewing the work of others. This is actually a respect problem, because fundamentally these people expect that others will review their work so it can get in but don't spend to time to do the same for others. With high time pressure people don't always see it that way but it is the truth. The approach here is to get people to understand this and that to ensure that the team is successful everyone needs to take time to review other peoples code so that everyone's code can get in. In general you shouldn't see much push back with this because most people want the team to be successful. There are always exceptions of course, like when a high priority issue has a hard deadline, but if you have people who think that their work is always more important than others then you have a different problem - a toxic team member that you need to get rid of.

Another problem is when particular people get swamped with PRs. Most of the time this is because they are considered the subject-matter-expert so everyone who touches that area adds them. You can also see the case where you get individuals that are just really good at PRs so people like adding them. What you need to do here is to identify and get everyone to acknowledge the issue before you can fix it. Then set guidelines like only including SME's if it's an important and/or risky PR. How do you know that? Ask them! Something I love is when people have a quick chat about a change and ask "do you want to review this?". SME's are in a great position to evaluate the risk factor and say "yes, I need to look at that" or "no, that sounds fine and ${DevX} can handle that review".

The last problem I've seen around this is when a bunch of people add too many people on each PR. Two heads are better than one, that's the whole point of PRs, but that doesn't mean you need five. I think you tend to see this more in junior people, as they're not sure who to add so they add everyone. And occasionally it can be good, like when trying to get feedback about a new approach to something. I personally did this recently with some React hooks code because a) we didn't have a well established pattern on how to use hooks yet, and b) I'm a React noob so I wanted to get some broad feedback. But that should be the exception, not the norm.

Collapse
 
alediaferia profile image
Alessandro Diaferia

Really valuable feedback, thank you very much.