On Fri, Feb 8, 2013 at 2:06 PM, Alex Huang <alex.hu...@citrix.com> wrote: > Hi everyone, > > I like to propose that we setup gerrit as the review mechanism. Here are my > reasons > > - Committer status in Apache is a reflection of one's commitment to the > community, not a reflection of understanding of code. So to me just because > you have committer status shouldn't mean code does not need review. Chip's > been doing a great job monitoring the merges and commits but one person > handling all that just means things will slip through. > - This also has the side effect of contributors' code contribution to be > treated as a second class citizen with delays in reviews because review is > not common place within our community. > - Direct commits have to be reverted if there are -1 votes, directly > impacting the time and effort of the code contributor. > > It makes a lot of sense to make code commit and review a normal process in > the CloudStack Community. >
So while I love Gerrit, and would love to see it implemented, most of what you've described above are social/cultural problems, not technical ones, and thus a technical solution is unlikely to fix those things. Gerrit also requires that there actually be tests in place to test, othewise it's just another hoop to jump through. With a whopping 3% of classes covered, it doesn't really mean much at this point, unless we spend significant amounts of time building tests. Speaking from a technical perspective - there is no ability to stop a committer from committing to the repo. And yes, committer status is a reflection of ones commitment to the project, and also of the trust we have in a person. That doesn't mean a committer is perfect. This is also a _dramatic_ change in culture - moving from a commit then review to a review then commit - this will dramatically slow down project development. I'll note that we have past guidance from our mentors that encouraged CTR as opposed to RTC. So let me toss out an example (and I am assuming you'll be requiring two committers/reviews to approve changes, because if we are just talking about publishing changes based on automated test infra, that's not much improvement over our current method due to our low test coverage ): If Hugo makes some packaging changes (he's made ~30 distinct commits around packaging within the past week - on two different branches (so 60 total commits)) he then needs to get two reviews for each of the those commits - which means he either has to work in isolation on his own to land massive changes that get reviewed by two people, or he has to get 120 reviews for his changes. Then lets talk about who is qualified to really review those commits. That's a dramatic subset of committers. And then I'd ask - what has the 120 reviews given us? Massive improvement? So in short, what we are really talking about is dramatically ramping up the number of reviews required to get something into ACS, and we've already demonstrated that we aren't capable of timely getting things reviewed in reviewboard, what makes us think that dramatically increasing the number of reviews needed to see progress is a good thing? -1 from me, though I am open to being persuaded, because I really do like Gerrit. --David
