On Wed, Nov 25, 2015 at 1:32 PM, Ralph Goers <ralph.go...@dslextreme.com> wrote:
> 1. What makes you think all bugs are caught during code reviews (they > aren’t)? > They aren't. But some are. And catching them in code review is cheaper than catching them when a user hits them. Additionally, plenty of other things are caught in code reviews other than bugs (style, compat issues, design issues, poor test coverage, etc) > 2. What makes you think that code reviews after the commit are any less > thorough than reviews required before the commit? > > If you don’t trust your community to do code reviews after you commit then > there is a problem in your community. Forcing a code review to occur first > won’t fix that. > Isn't it an issue of scalability? With pre-commit code reviews, typically the uploader of the code will pick out one or two people to review the code who know the area well. Or, if no one is picked by the submitter of the patch, the committers will organically end up deciding who is to review the code, at which point that reviewer ends up being a sort of shepherd for the patch, sticking with the contributor through multiple revs until it's ready for commit. With post-commit review, do you expect to watch the mailing list and review every patch that comes in? In a project like Hadoop, that's not feasible -- we've had ~35,000 lines of code changed in the last month in 267 patches. If everyone tries to review every patch post-commit, you end up with n^2 work as the community grows. Amusingly enough, I happened upon a chapter in "Producing Open Source Software" that invoke's Greg's name on the subject of open source code review (http://producingoss.com/en/setting-tone.html): There was no guarantee that every commit would be reviewed, though one > might sometimes look over a change if one were particularly interested in > that area of the code. Bugs slipped in that really could and should have > been caught. A developer named Greg Stein, who knew the value of code > review from past work, decided that he was going to set an example by > reviewing every line of every single commit that went into the code > repository. Each commit anyone made was soon followed by an email to the > developer's list from Greg, dissecting the commit, analyzing possible > problems, and occasionally praising a clever bit of code. I'm impressed that Greg was able to do this with Subversion, but not sure how it could work in a faster paced project, and also feel like this practice produces a serious "bus factor" issue. -Todd
