Edison, My understanding of our process is that the merges of significant changes should be proposed to the mailing list with the "[MERGE]" tag and wait up to 72 hours for feedback. I consider interface changes to meet that criteria given the potential to break other folks work. It sounds like we had a change that inadvertently slipped through without notice to list. Going forward, I propose that we follow this process for significant patches and, additionally, push them to Review Board. As a matter of collaboration, it might also be a good idea to open a "[DISCUSS]" thread during the design/planning stages, but I don't think we need to require it.
Do you think this approach will properly balance to our needs to coordinate/review work with maintaining a smooth flow? Thanks, -John On Jun 21, 2013, at 2:15 PM, Edison Su <edison...@citrix.com> wrote: > > >> -----Original Message----- >> From: Chip Childers [mailto:chip.child...@sungard.com] >> Sent: Thursday, June 20, 2013 5:42 PM >> To: dev@cloudstack.apache.org >> Cc: Edison Su >> Subject: Re: [DISCUSS] Do we need code review process for code changes >> related to storage subsystem? >> >> On Thu, Jun 20, 2013 at 05:59:01PM +0000, Edison Su wrote: >>> For interface/API changes, we'd better have a code review, as more >> storage vendors and more developers outside Citrix are contributing code to >> CloudStack storage subsystem. The code change should have less surprise >> for everybody who cares about storage subsystem. >> >> I'm not following what you are saying Edison. What are we not doing in this >> regard right now? I'm also not getting the "Citrix" point of reference here. > > We don't have a code review process for each commit currently, the result of > that, as the code evolving, people add more and more code, features, bug > fixes etc, etc. Then maybe one month later, when you take a look at the code, > which may be quite different than what you known about. So I want to add a > code review process here, maybe start from storage subsystem at first. > The reason I add "Citrix" here, let's take a look at what happened in the > last month: > Mike, from SolidFire, is asking why there is a hypervisor field in the > storage pool, simply, the hypervisor field breaks his code. > And I don't understand why there is a column, called dynamicallyScalable, in > vm_template table. > I think you will understand my problem here... > > > >> >> -chip
