How should my team distribute checking pull requests?

Question

I'm working as part of a team of 10 on an agile project, this is our first time using GitHub. We write our code, create a pull request and then start on our next task. 95% of these pull requests are being reviewed by our scrummaster/manager who tests whether everything is working but generally doesn't look at the code. Another issue is that he's a bit overloaded so PRs can take several days to be seen and if the code is important I'll sometimes just merge immediately after testing it myself.

I'm definitely guilty of not bothering to check other people's pull requests as it's quite disruptive to immediately stop what I'm working on to look at code that has nothing to do with what you're working on. I'd also get considerably less done than everyone else if I started to accept PRs from all corners.

Who should accept responsibility for reviewing someone's pull requests and how can we ensure this responsibility is not laid upon the same people all/most of the time?

possible duplicate of Can a developer perform testing efficiently? — gnat, Aug 09 '16 at 19:45
@gnat, I don't think this is a dupe. Pull requests are as much about code review and coordination of development effort as they are about testing. The OP states he's looking at the code, not merely testing the solution. — Tim Grant, Aug 09 '16 at 19:54
I did edit out a sentence at the end asking if we needed a dedicated tester as it's a 2nd question really and not primarily what I want to find out. — Declan McKenna, Aug 09 '16 at 19:58
"We have a 'Code Review' task in each story. Someone ideally not involved in the development of that story will review all code changes associated with that story. It works well.
A lot of time? Not very much..." (Should we attempt to review all our code?) — gnat, Aug 09 '16 at 21:01
Can't you assign the reviews to people who work in most related parts of the code base? That way they will have more connection with the part because it influences their work. Also: are all developers equal? No juniors/seniors structure? — Luc Franken, Aug 10 '16 at 07:41

score 7 · Accepted Answer · answered Aug 09 '16 at 21:45

7

A team of 10 is approaching the large side for an agile team, but you can still keep the pull requests manageable if you make a rule that people can merge after two thumbs ups. That means for every pull request you create, on average you are reviewing two, assuming you don't get many from outside your team.

That's not an unreasonable rate, and usually once your team gets out of the habit of sticking it to one person, it becomes a lot easier. To my knowledge, everyone on my team has a bookmark of recently updated pull requests that they clear twice a day or so when they need a break from their own work. When someone needs a review more urgently, they make a special request. It's not that hard.

answered Aug 09 '16 at 21:45

Karl Bielefeldt

147,435

2

It is hard once the team has gotten in the habit of letting one person do it all. Breaking that bad habit is damn hard actually. – RubberDuck Aug 10 '16 at 00:35
@RubberDuck: I think Karl meant "hard" as in "difficult" or "complicated". His process is simple. Doesn't mean it is easy... – Marjan Venema Aug 10 '16 at 10:27
Twice a day! Part of our problem may be the size of our PRs, we create a PR for each story/feature so we have infrequent but large PRs. – Declan McKenna May 03 '17 at 21:15

How should my team distribute checking pull requests?

1 Answers1

Linked