>In terms of speed, we find that developers have to wait for initial feedback on their change a median time of under an hour for small changes and about 5 hours for very large changes. The overall (all code sizes) median latency for the entire review process is under 4 hours
I worked at Google from 2014-2018. I'm extremely skeptical of these numbers.
Based on my experience working on three different teams and sending changelists to many other teams outside my own, I'd expect something more like 3-4 hours for small changes (<50 LOC) and 1-2 days for large changelists (500+ LOC).
Google has so many automated changelists that I'm wondering if that skewed their numbers. Like if someone just clicks through 10 auto-generated CLs and approves them all, did that count for a bunch of near-instant review cycles?
That said, I was a big fan of Google's code review process. I know people complained about the unnecessary level of process and the hassle of getting readability, but I found it all a great way to spread knowledge across teammates and improve programming skills.
I think pocket reviewers probably account for the generally short time. In my day-to-day I usually did not send changes to people who weren’t already primed to receive them and sitting right next to me, so I could turn around and say “I sent you the thing” and they’d review it straight away because we were working towards the same goal anyway, and in some cases had already pair-designed or pair-coded the thing so the review was already a formality.
Caution though: this can lead to blind spots. I witnessed a critical bug become exacerbated by the first fix because the developer and reviewer had essentially co-wrote the solution, so the sign-off was essentially a rubber stamp.
If you want the least bias, you should find someone to review something that hasn't been deeply involved in designing it. And for pieces of code that are small enough, reading and getting up to speed shouldn't take that much time. I prefer sending reviews to engineers that are familiar with the area of the code, but we don't do pair-programming so the implementation hasn't been seen before.
We do, however, do a lot of design work before a single line of code is written.
Yep, "pocket reviewer" generally has a negative connotation. But, let's face it, most changes are minor. You have to be judicious about the level of review required for a given changelist.
That's definitely curious. If the median latency is under 4 hours, does that mean there are lots of people sitting around doing nothing, or working on nothing urgent, so that they can immediately turn around a code review?
> does that mean there are lots of people sitting around doing nothing, or working on nothing urgent, so that they can immediately turn around a code review?
No, it means there is a cultural expectation / etiquette that you prioritize keeping your teammates unblocked by reviewing their code quickly, even if it means putting aside your own coding to do so.
In addition to what the other user said, most changes are also small. Google's infra and culture encourages changes that are the size of a commit, not a pull request.
This depends on the particular language and context, but simple, isolated, and usually small changes are preferred and encouraged. This allows relatively quick review for lots of things.
If I see a change that modifies a single function and adds a test to validate the change in functionality, that's very quick to review.
I worked at Google from 2014-2018. I'm extremely skeptical of these numbers.
Based on my experience working on three different teams and sending changelists to many other teams outside my own, I'd expect something more like 3-4 hours for small changes (<50 LOC) and 1-2 days for large changelists (500+ LOC).
Google has so many automated changelists that I'm wondering if that skewed their numbers. Like if someone just clicks through 10 auto-generated CLs and approves them all, did that count for a bunch of near-instant review cycles?
That said, I was a big fan of Google's code review process. I know people complained about the unnecessary level of process and the hassle of getting readability, but I found it all a great way to spread knowledge across teammates and improve programming skills.