incubator-general mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Todd Lipcon <>
Subject Re: RTC vs CTR (was: Concerning Sentry...)
Date Wed, 25 Nov 2015 22:22:05 GMT
On Wed, Nov 25, 2015 at 1:32 PM, Ralph Goers <>

> 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 (

 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.


  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message