I have a few comments: 1. The table autoscale_vmprofiles has account_id as well as user_id. Only account_id should be sufficient? 2. The table 'counter' and 'condition' should be renamed so that it is clear that it is autoscaling related. So also the corresponding Java classes, APIs and interfaces 3. Any indexes at all on these new tables? 4. autoscale_vmprofiles (and java class) should not have reference to snmp. We discussed why before, and alternatives to this design on this mailing list. 5. A lot of the changes to existing files are whitespace changes that are unrelated to the logic. Please avoid this. 6. Why is a Counter not a ControlledEntity but Condition is? 7. Not sure why the LoadBalancer* classes have been touched. Everything should be isolated to Autoscale? Can you explain why you had to make API changes to the LoadBalancingRulesManager? 8. The License header is not consistent (some with leading spaces, others without) 9. Not all files have the Apache license.
On 8/14/12 9:27 AM, "Ram Ganesh" <ram.gan...@citrix.com> wrote: >> -----Original Message----- >> From: Ewan Mellor [mailto:ewan.mel...@eu.citrix.com] >> Sent: 08 August 2012 05:42 >> To: cloudstack-dev@incubator.apache.org >> Cc: Pranav Saxena; David Nalley; Vijay Venkatachalam; Alena >> Prokharchyk; Deepak Garg >> Subject: RE: where features are developed was: Review Request: Merge >> Kelven's VPC code for Vmware into asf vpc branch >> >> > -----Original Message----- >> > From: David Nalley [mailto:da...@gnsa.us] >> > Sent: Tuesday, August 07, 2012 3:42 PM >> > To: cloudstack-dev@incubator.apache.org >> > Subject: Re: where features are developed was: Review Request: Merge >> > Kelven's VPC code for Vmware into asf vpc branch >> > >> > On Tue, Aug 7, 2012 at 4:59 PM, Ewan Mellor >> <ewan.mel...@eu.citrix.com> >> > wrote: >> > > That makes a lot of sense to me, thanks Chip. >> > > >> > > For those public repos for non-committers, I presume that they >> can't >> > use Apache infrastructure, so Github would be the natural choice. We >> > have a mirror there now (as of last week) so people can clone and >> work >> > there. How does that sound? >> > > >> > > Regarding the third point, I don't know what the ASF requires in >> > terms of authorship indicators in commit messages. We obviously need >> > all authors to have signed the ICLA, but other than that I don't know >> > of a written policy. Unless there's something contradictory, putting >> > the Author names in the commit message seems reasonable to me. >> > >> > >> > The below has some guidance: >> > http://apache.org/dev/committers.html#applying-patches >> > >> > Essentially some annotation needs to be made, but there is no >> absolute >> > practice; so comments are fine. >> >> Thanks David, that looks good to me. >> >> So can we give this process the first go through, using the autoscale >> branch? We've got the branch ready to go, if I understand correctly, >> so we just need Vijay or Deepak to make sure that the authors are all >> appropriately credited (and that everyone has signed the ICLA). Then >> we should be able to start reviewing that branch for merge into 4.0. >> >> Ewan. > >Ewan: By autoscale branch I am assuming you are referring to topic branch >in ASF. Please correct if I got it wrong. Also autoscale feature authors >have also signed the ICLA. Are we good to go now? > >Thanks, >Ram >
