[editing subject since the discussion has veered away from Sentry] On Mon, Nov 16, 2015 at 7:56 AM, Ralph Goers <ralph.go...@dslextreme.com> wrote:
> And I have to disagree with you Joe. To me, a mandatory RTC policy says > “we don’t trust anybody”. Sure, it doesn’t discriminate, but it is also a > PITA. One project I mentored uses RTC along with ReviewBoard and mandates > that you cannot commit your own work and every commit must be formally > reviewed. I have found this process to be so onerous that I have never > committed any code to the project, even though I really would like to. I > find the pace of this project to be fairly slow. But it seems to fit > within the corporate culture that most of the committers seem to work in. > > OTOH, I am involved in a project that uses CTR but where feature branches > are frequently created to allow others to review and improve significant > new work before it is integrated. As a consequence, new features are > introduced at a much faster pace in this project. > I've seen this RTC vs CTR discussion come up a number of times over the last ~6 years that I've been involved in ASF projects. For every strong opinion in favor of CTR (like yours) there is a strong opinion in favor of RTC (like mine). The quick summary of my viewpoint is: 1) You're right, I don't trust anybody to make code changes to a complex project with zero oversight. I currently work on a project that I originally started, and was the only engineer on for a few months. Even when I make changes to code that I wrote 100% of the module, I get others to review my work, and you know what? It turns out better for it. Sometimes they find bugs. Often they find areas where I didn't comment the code well, design it as well as I could have, or missed potential performance optimizations. Coding's hard. Coding on complex projects is even harder. Some projects aren't complex, and maybe on those a CTR policy makes a lot more sense. But distributed systems, database storage engines, etc, are pretty hard to get right, even for the "experts". I'm always glad to have a second pair of eyes before I introduce a bug that takes down critical infrastructure. 2) Regardless of trust, code review helps build _shared ownership_ of code. In community projects, without code review, it's easy to end up with "pet features" or areas of code that only one person understands. When that person moves on to new employment or new interests, the project is stuck with a bunch of source code that no one understands how to maintain. Forcing the original author to get some reviews before committing ensures that there is a more likely path to project continuity after they move on to new pastures. 3) Code reviews are a great way for engineers to learn from others in the community. Earlier in my career, I certainly learned a lot from the committers on projects like Hadoop and HBase where I "cut my teeth". And even as a long-time committer on these systems, I still learn new things from reviewing code written by newer members of the community. Review is where a lot of the cross-contributor interaction takes place, so it helps build a cohesive community. Given #2 and #3, I see RTC as an extension of "community over code". Sure, it might slow down the introduction of a new feature or fix to have to wait to get a review from another community member. But, that's just code. Getting more eyes and hands on the code builds the community. After writing the above, I started to feel it was familiar and remember a very similar discussion on hbase-dev last year: http://mail-archives.apache.org/mod_mbox/hbase-dev/201508.mbox/%3CCA+RK=_dz+_rzumfwvya0tkvxtk4saeie6pwpgs2mxvsbq8h...@mail.gmail.com%3E - I'd recommend people go check that one out to see the various viewpoints. I don't anticipate that the above arguments will convince you, or anyone else who believes in CTR, to change your mind. But, as mentioned in some other recent incubator threads, let's not use the incubator as a battleground for personal opinions. There are successful Apache projects following all sorts of development models, and the important thing is only that (a) projects build inclusive communities, and (b) projects follow proper licensing/release/etc processes for legal reasons. -Todd
