incubator-cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chiradeep Vittal <Chiradeep.Vit...@citrix.com>
Subject Re: where features are developed was: Review Request: Merge Kelven's VPC code for Vmware into asf vpc branch
Date Wed, 15 Aug 2012 02:02:10 GMT
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.Ganesh@citrix.com> wrote:

>> -----Original Message-----
>> From: Ewan Mellor [mailto:Ewan.Mellor@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:david@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.Mellor@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
>


Mime
View raw message