If you have three reviewers, no one feels personally responsible for checking it line by line and thinking about the code in depth. Instead a cursory glance seems acceptable, because, after all, other people are looking at it.
> If you have three reviewers, no one feels personally responsible for checking it line by line and thinking about the code in depth.
in my experience, this is already happening. When you get pinged to review PR after PR and have your own work to do, it's easy to skim. Requiring 2 reviewers operationally has often meant i'd get a chance to look and take a deep dive after someone caught the glaring issues.
When it's ok for just one dev to sign-off, time and time again that's how i've seen bugs/debt sneak in. I think giving organizations this option is only a plus, and is not really something we need to have a public debate about; it's an organizational decision to use or not use this that github has facilitated.
Two reviewers seems optimal if you can afford it. Possibly with one person doing the bulk of the reviewing and the other more in the role of providing oversight and breaking ties if they occur.
What I’ve decided is that it’s folly to try to enforce accountability. A good system provides visibility, that’s it. Who did what? Document it, but allow implicit social process to handle enforcement. Hard rules just lead to people cooking the books, or opting out of the system entirely.
I feel the same about code review. Document who reviewed. But it’s up to individuals to take responsibility for the outcome. And at the end of the day it’s the committer’s code. It’s their name in the git history.
Companies have managers. It's very important that mangers at a company set the tone for using mistakes to create better engineering processes. And that involves a lot more than shaming the committer.
I developed an app (https://pullreminders.com) that helps teams stay on top of open pull requests and in talking to customers, there is a huge demand for features and tools to support a better code review process.
(Disclaimer: I am the creator of Pull Reminders)
In my experience, bugs pop up more often in changes where reviewers aren't honest in how well they actually understand the changes, and are maybe afraid of admitting their confusion (even though it's usually the case that if one person in the room is confused, everyone is).
This change allows you to prevent that from happening, by requiring as many reviewers to approve a request as your team has—so that everyone has to approve it. Previously, GitHub only let you set one required reviewer, which enabled diffusion of responsibility.
The responsibility (or blame?) for bugs is diffused over more people, so people will care less.
I'm not sure that I agree, but I can at least understand the argument.
There is no way I am looking at every single line, and even if I did it doesn't mean I understand the code.
Having two people check something over is a good thing, telling two people that they should both start simultaneously to do work that might be duplicate is bad.
Chrome/Google have something like that but more flexible (e.g. you can have nested OWNERS files) called OWNERS https://chromium.googlesource.com/chromium/src/+/master/docs...
There's a more general/kinda crazy system in Gerrit (by Google) that let's you write custom rules in Prolog (which helps for solving for a minimal set of reviewers that could approve a given PR.) I'm not sure if it gets much use but its a neat thought for sure: https://gerrit-review.googlesource.com/Documentation/prolog-... (which links to this email explaining why: https://groups.google.com/d/msg/repo-discuss/wJxTGhlHZMM/Tal... )
I think the constraint-solver method in Gerrit is pretty neat, although I could see integrating that into GitHub and devising a non-painful UI for that as a major challenge. However, if they managed to do it, that would be an insanely powerful feature to have, especially for larger or more complex projects.
From the title, I initially thought this would be an article about why you should have multiple people reviewing PRs, which sounded ridiculous (obviously we don't all have the time/resources for that).
I think perhaps they should have made it infinite, because having a set range causes anchoring. It might seem like 3 is a good choice because 2 is at the low end of things.
