1

We are using microservices(read a lot of GIT repositories) and our department has 90+ java developers. All of them write code which mean there are a lot of PR`s every day. We are using BitBucket and Also we have a self written bot that listen BB events and can sends a PR review notification to slack.

Notify every developers about each created PR looks like PR DDOS. So we configure bot to use two groups: mandatory approvers(PR can't be merge without their approve) and others. There were about 15 mandatory approvers. In this case each PR has to be approved by 1 mandatory and 3 other reviewers to be merged. But daily amount of PR's is too big and mandatory approvers still got a lot of notifications which stops their main work. It was our first approach and we decided to declined it.

The second approach is to split all developers by groups(15-20 devs per group) within related business area and don't use mandatory approvers. In this case each PR has to be approved by five reviewers. After that it's ready to merge. Also such attempt should increase overall expertise cause devs from one business are can review PR more thoroughly.

Right now bot is sending notification to all participant from group to which author belongs. It spread load to all devs from group. The amount of PR's to review changes day by day but usually it is still quietly big.

Maybe there are another approaches use in your company. Just curios to know. Maybe you have some ideas how to rebuild review process.

One of the next steps we are thinking about is to decrease the number of devs in one group which decrease daily amount of PR's for one dev.

Improve this question
6
  • 2
    What is your field of work? 4-5 reviewers is probably ok (and maybe even required) for medical or aerospace code...
    – Alexei Levenkov
    Sep 16, 2021 at 5:34
  • 1
    5 reviewers seems a lot to me, even in the formentioned fields - can you elobarate on this?
    – Doc Brown
    Sep 16, 2021 at 6:05
  • 1
    Yep, maybe 5 reviewers is too much and we can decrease it to 3. We are developing a bank software. The number 5 was like a random choice for first iteration.
    – Mirian
    Sep 16, 2021 at 7:51
  • 1
    I worked on a (modular) monolith project with 40-ish developers. Developers were divided into 6 teams. Each team owned some modules. We only required 2 reviewers for internal changes and an additional one from the external team in case we wanted to change something that did not belong to the current team scope.
    – Hieu Le
    Sep 16, 2021 at 7:53
  • 1
    @Mirian we didn't get notifications for new PRs. We used Jira board to see if any tickets waited for another review. There were no pre-assigned reviewers unless in specific tickets that the whole team agreed (in the planning meeting) that a specific person in the team is the mandatory reviewer. In short, only people involved in the PR (the developer, the reviewers, and one have comments) are notified about the changes of the PR.
    – Hieu Le
    Sep 16, 2021 at 9:06

3 Answers 3

Reset to default
7

In this case each PR has to be approved by five reviewers.

This is your biggest problem. Reduce this number to two or at absolute maximum, three. If you don't trust that two developers on your team can effectively review code from another developer on your team, get better reviewers.

(15-20 devs per group)

This is your second biggest problem. Your teams should be aligned around your microservices, and the team responsible for each microservice should be reviewing the code for that service; they are the domain experts after all. That means a maximum of say 6-7 people in each group ("two pizza team") - give them actual ownership of each service, rather than trying to distribute it around your team of 90 developers.

Improve this answer
3
  • 1
    The term Domain Expert typically refers to someone working in that particular domain. It’s the person developers go to with questions. Surely developers will gain a lot of knowledge about a particular domain when creating software for it, but that doesn’t make them domain experts.
    – Rik D
    Sep 16, 2021 at 5:59
  • 1
    @RikD Thinking of microservices as small, bounded domains here.
    – Philip Kendall
    Sep 16, 2021 at 8:43
  • 1
    I agree with @RikD, even in high autonomous microservice teams, developers are not domain experts. They are technical experts, they know much about their current domain, but they are not the person who makes final decisions if business logic is implemented correctly.
    – Hieu Le
    Sep 16, 2021 at 9:09
2

If every pull request has five reviewers, then the average developer’s workload includes five reviews for every pull request they produce. That’s a lot of time in my day spent on pull requests.

I wonder how much benefit you get from that.

Improve this answer
1
  • 1
    Yep, looks like 5 is too much. It was our first iteration. Will decrease it to 3.
    – Mirian
    Sep 16, 2021 at 8:33
0

Apart from the number of reviewers, the way developers are notified about PRs might be an issue. Interruptions are bad. Actually, REALLY BAD, in bold uppercase.

Try to find ways that developers can switch from "development mode" into "review mode" and get PRs to review only when in review mode. Don't interrupt their development work. It might be good for each developer to make that switch only once or very few times per day. You need to decide whether reviewers are free to pick PRs (for example those covering areas where they have most expertise, or those from new colleagues who may need some guidance about the product) or assign using some random (and hopefully fair) scheme.

Improve this answer

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.