On Thu, Nov 19, 2015 at 8:16 AM, Ralph Goers <ralph.go...@dslextreme.com> wrote:
> None of your statements below are any different between RTC or CTR. The > only time it makes aa difference is if no one does reviews. My feeling is > that a community that insists on RTC believes that code will not be > reviewed unless committers are forced to do it. > Yep, that's my assumption. It's much more fun to write code than review it, so "forcing" people to do it is a good idea. The other worry is that, in a large community of developers, an implicit "people probably read the commits as they come in" doesn't scale. Should every person read every commit? Probably not. How do you know if someone else has already read the commit, or signed up to do so? What if it takes you a few days to get around to reviewing something, and by that point there are already a bunch more patches stacked up on top making it impossible to revert or difficult to modify? Who's in charge of fixing the bugs or issues in a post-commit review? I'm sure it works fine for many communities (I use CTR on some internal infrastructure within small teams where bugs are less costly and the rate of development is slow). But it doesn't work for all. > > All I can say, is that for me personally I have found the process of > having to create a patch, submit a code review, wait for the review and > participate in it, then wait for the commit to be onerous enough that I > just don’t bother. Sure, that's a big problem with some RTC workflows. Using gerrit or github PRs makes the flow much easier -- for a trivial or small patch, the sort that a "drive-by" contributor typically contributes, there probably won't be any review comments. So, they just push the patch for review, and they can be out of the loop for the rest of it. If the patch requires small revisions (eg addressing a typo or something) I think it's fine for the reviewer to just make the change themselves and commit on behalf of the original author to avoid the issue you've raised. Most RTC workflows permit this kind of thing in my experience. > As I said, in a CTR community there are many times where branches are > created and the code is reviewed there before being merged because the > authors believe the code is significant enough to require it. Amusingly enough, the RTC communities I'm a part of do the opposite: you can make a branch which operates under CTR, so long as it's reviewed sufficiently prior to its merge into trunk. This is great for rapid development and prototyping when a small number of contributors are working together on a new project. -Todd -- Todd Lipcon Software Engineer, Cloudera
