And if each part of the review takes 2 days in the first place, there might be an issue with your code reviews (or your checkins are gargantuan).
And if each part of the review takes 2 days in the first place, there might be an issue with your code reviews (or your checkins are gargantuan).
We switch to pre-commit reviews instead of the usual post-commit review flow if the potential errors/mistakes are costly and can cause significant problems. Some examples of such cases are:
Touching code which deals with privacy and anonymity of users.
Touching a core abstraction since a lot of other code might depend on it.
Touching some parts of the infrastructure where mistakes can cause a downtime.I would not move to this workflow, although it's interesting to hear from somewhere that it works.
While post commit review might work for smaller changes I would rather err on the side of caution and use precommit review. We deploy about the same number of times/day/engineer at my company, so I don't think that the review process is what drives that metric.
But I'm really curious - if you can talk about it, that is - why devs push on average almost 4 changes per day & dev, but a review takes 2 days roundtrip.
I'm highly skeptical of code reviews as the way to find bugs. I think code reviews have other values, just not that one so much. Getting multiple eyes on code and the results of that code, getting it into the hands of QA, dog fooding it, all play a role. That all sounds way more ad-hoc than I mean it. I used to write life critical software, and you have well defined ways to handling source control and reviews. I advocate figuring out what your project needs, adopting that, and then continually adapting as the project changes, as any project will.
At Quora, we generally do post-commit code reviews. That is, the code goes out live in production first and someone comes and reviews the code later.
Strangely, this didn't hold for major bugs, which were often done by newbies to the codebase. Major bugs are often created by not understanding the implications of a change or the other systems that might be impacted; experienced devs usually know this by heart, so they don't screw it up in their designs.
I've heard of this process ("bebugging", the opposite of "debugging") being used for statistical QA analysis. If you purposely inject 10 bugs (don't forget to remove them! :) and QA or fuzzing finds 7 of them plus (say) 42 other bugs, then you can extrapolate that you have about 18 undiscovered bugs (10/7*42-42 = 18). This assumes your 10 injected bugs are representative of the actual bugs you find, which is harder than it sounds.