You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Daan Hoogland <da...@gmail.com> on 2015/07/08 13:41:01 UTC

revert or not revert? (was: [GitHub] cloudstack pull request: CLOUDSTACK-8583 : fixing issue related to...)

guys and dolss,

this was closed with only 1 LGTM! we agreed not to submit with less
then two ok reviews on master. So next steps? revert? some extra
justification or else revert? .....

On Wed, Jul 8, 2015 at 12:39 PM, asfgit <gi...@git.apache.org> wrote:
> Github user asfgit closed the pull request at:
>
>     https://github.com/apache/cloudstack/pull/567
>
>
> ---
> If your project is set up for it, you can reply to this email and have your
> reply appear on GitHub as well. If your project does not have this feature
> enabled and wishes so, or if the feature is enabled but not working, please
> contact infrastructure at infrastructure@apache.org or file a JIRA ticket
> with INFRA.
> ---



-- 
Daan

Re: revert or not revert? (was: [GitHub] cloudstack pull request: CLOUDSTACK-8583 : fixing issue related to...)

Posted by Sebastien Goasguen <ru...@gmail.com>.
> On Jul 9, 2015, at 3:27 PM, David Nalley <da...@gnsa.us> wrote:
> 
> On Thu, Jul 9, 2015 at 4:49 AM, Rohit Yadav <ro...@shapeblue.com>
> wrote:
> 
>> 
>> On 09-Jul-2015, at 12:50 pm, Sebastien Goasguen <ru...@gmail.com> wrote:
>> 
>> Let’s revert , that’s our new rule
>> 
>> 
>> Since we have not voted what/how we are going to do commits, let’s treat
>> this more of a guideline or honour code.
>> 
> 
> 
> Votes aren't strictly necessary. As a matter of fact, votes should be
> avoided if at all possible. We should strive for consensus, and when we
> have it, it is as binding as if we had a tally of votes.
> 
> 
>> 
>> I’ve expressed few concersn on this rule wrt code reviewing for PRs that
>> are not getting attention, so IMO let’s try to have the discussion and find
>> solution before we really start enforcing them.
>> 
>> 
> So my take (FWIW) is that without the pain, we'll never adjust our method
> of operation or find the solutions. In example, no one came to the list
> asking for reviews.
> 

+1


> --David

Re: revert or not revert? (was: [GitHub] cloudstack pull request: CLOUDSTACK-8583 : fixing issue related to...)

Posted by David Nalley <da...@gnsa.us>.
On Thu, Jul 9, 2015 at 4:49 AM, Rohit Yadav <ro...@shapeblue.com>
wrote:

>
> On 09-Jul-2015, at 12:50 pm, Sebastien Goasguen <ru...@gmail.com> wrote:
>
> Let’s revert , that’s our new rule
>
>
>  Since we have not voted what/how we are going to do commits, let’s treat
> this more of a guideline or honour code.
>


Votes aren't strictly necessary. As a matter of fact, votes should be
avoided if at all possible. We should strive for consensus, and when we
have it, it is as binding as if we had a tally of votes.


>
>  I’ve expressed few concersn on this rule wrt code reviewing for PRs that
> are not getting attention, so IMO let’s try to have the discussion and find
> solution before we really start enforcing them.
>
>
So my take (FWIW) is that without the pain, we'll never adjust our method
of operation or find the solutions. In example, no one came to the list
asking for reviews.

--David

Re: revert or not revert? (was: [GitHub] cloudstack pull request: CLOUDSTACK-8583 : fixing issue related to...)

Posted by Rohit Yadav <ro...@shapeblue.com>.
On 09-Jul-2015, at 12:50 pm, Sebastien Goasguen <ru...@gmail.com>> wrote:

Let’s revert , that’s our new rule

Since we have not voted what/how we are going to do commits, let’s treat this more of a guideline or honour code.

I’ve expressed few concersn on this rule wrt code reviewing for PRs that are not getting attention, so IMO let’s try to have the discussion and find solution before we really start enforcing them.

Regards,
Rohit Yadav
Software Architect, ShapeBlue


[cid:9DD97B41-04C5-45F0-92A7-951F3E962F7A]


M. +91 88 262 30892 | rohit.yadav@shapeblue.com<ma...@shapeblue.com>
Blog: bhaisaab.org<http://bhaisaab.org> | Twitter: @_bhaisaab




Find out more about ShapeBlue and our range of CloudStack related services

IaaS Cloud Design & Build<http://shapeblue.com/iaas-cloud-design-and-build//>
CSForge – rapid IaaS deployment framework<http://shapeblue.com/csforge/>
CloudStack Consulting<http://shapeblue.com/cloudstack-consultancy/>
CloudStack Software Engineering<http://shapeblue.com/cloudstack-software-engineering/>
CloudStack Infrastructure Support<http://shapeblue.com/cloudstack-infrastructure-support/>
CloudStack Bootcamp Training Courses<http://shapeblue.com/cloudstack-training/>

This email and any attachments to it may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Shape Blue Ltd or related companies. If you are not the intended recipient of this email, you must neither take any action based upon its contents, nor copy or show it to anyone. Please contact the sender if you believe you have received this email in error. Shape Blue Ltd is a company incorporated in England & Wales. ShapeBlue Services India LLP is a company incorporated in India and is operated under license from Shape Blue Ltd. Shape Blue Brasil Consultoria Ltda is a company incorporated in Brasil and is operated under license from Shape Blue Ltd. ShapeBlue SA Pty Ltd is a company registered by The Republic of South Africa and is traded under license from Shape Blue Ltd. ShapeBlue is a registered trademark.

Re: revert or not revert? (was: [GitHub] cloudstack pull request: CLOUDSTACK-8583 : fixing issue related to...)

Posted by Sebastien Goasguen <ru...@gmail.com>.
> On Jul 9, 2015, at 4:45 AM, Rajani Karuturi <ra...@apache.org> wrote:
> 
> PR #565 is also closed with one review.
> 
> I dont agree to adding an exception. Reviewing would be one way to get
> others engaged and become aware. Otherwise, they lay there untouched and
> unused.
> 

Let’s revert , that’s our new rule

> I am sure we have enough committers who can review. Lately, lot more folks
> are adding marvin tests as well.
> 
> sailaja,sowmya,talluri,gaurav Can you review the PRs?
> 
> Regarding the revert,
> Maybe we can wait for 8 more hours (to catch up on IST and other timezones)
> to get another review. If we dont get any review or if we get a -1, we
> should revert.
> 
> 
> ~Rajani
> 
> On Thu, Jul 9, 2015 at 4:30 AM, David Nalley <da...@gnsa.us> wrote:
> 
>> More tests are good, on the other hand exceptions tend to beget
>> exceptions at tremendous speed.
>> 
>> On Wed, Jul 8, 2015 at 7:48 AM, Daan Hoogland <da...@gmail.com>
>> wrote:
>>> That was never formalized, hence I put it as a shakespearian question.
>>> I did sugest it otherwise I would have just reverted. I don't like the
>>> exception but if the number of people working on integration is to low
>>> to have a good flow we could.
>>> 
>>> On Wed, Jul 8, 2015 at 1:46 PM, Sanjeev N <sa...@apache.org> wrote:
>>>> I thought there is a limitation on LGTMs for integration tests?
>>>> 
>>>> On Wed, Jul 8, 2015 at 5:11 PM, Daan Hoogland <da...@gmail.com>
>>>> wrote:
>>>> 
>>>>> guys and dolss,
>>>>> 
>>>>> this was closed with only 1 LGTM! we agreed not to submit with less
>>>>> then two ok reviews on master. So next steps? revert? some extra
>>>>> justification or else revert? .....
>>>>> 
>>>>> On Wed, Jul 8, 2015 at 12:39 PM, asfgit <gi...@git.apache.org> wrote:
>>>>>> Github user asfgit closed the pull request at:
>>>>>> 
>>>>>>    https://github.com/apache/cloudstack/pull/567
>>>>>> 
>>>>>> 
>>>>>> ---
>>>>>> If your project is set up for it, you can reply to this email and
>> have
>>>>> your
>>>>>> reply appear on GitHub as well. If your project does not have this
>>>>> feature
>>>>>> enabled and wishes so, or if the feature is enabled but not working,
>>>>> please
>>>>>> contact infrastructure at infrastructure@apache.org or file a JIRA
>>>>> ticket
>>>>>> with INFRA.
>>>>>> ---
>>>>> 
>>>>> 
>>>>> 
>>>>> --
>>>>> Daan
>>>>> 
>>> 
>>> 
>>> 
>>> --
>>> Daan
>>

Re: revert or not revert? (was: [GitHub] cloudstack pull request: CLOUDSTACK-8583 : fixing issue related to...)

Posted by Rajani Karuturi <ra...@apache.org>.
PR #565 is also closed with one review.

I dont agree to adding an exception. Reviewing would be one way to get
others engaged and become aware. Otherwise, they lay there untouched and
unused.

I am sure we have enough committers who can review. Lately, lot more folks
are adding marvin tests as well.

sailaja,sowmya,talluri,gaurav Can you review the PRs?

Regarding the revert,
Maybe we can wait for 8 more hours (to catch up on IST and other timezones)
to get another review. If we dont get any review or if we get a -1, we
should revert.


~Rajani

On Thu, Jul 9, 2015 at 4:30 AM, David Nalley <da...@gnsa.us> wrote:

> More tests are good, on the other hand exceptions tend to beget
> exceptions at tremendous speed.
>
> On Wed, Jul 8, 2015 at 7:48 AM, Daan Hoogland <da...@gmail.com>
> wrote:
> > That was never formalized, hence I put it as a shakespearian question.
> > I did sugest it otherwise I would have just reverted. I don't like the
> > exception but if the number of people working on integration is to low
> > to have a good flow we could.
> >
> > On Wed, Jul 8, 2015 at 1:46 PM, Sanjeev N <sa...@apache.org> wrote:
> >> I thought there is a limitation on LGTMs for integration tests?
> >>
> >> On Wed, Jul 8, 2015 at 5:11 PM, Daan Hoogland <da...@gmail.com>
> >> wrote:
> >>
> >>> guys and dolss,
> >>>
> >>> this was closed with only 1 LGTM! we agreed not to submit with less
> >>> then two ok reviews on master. So next steps? revert? some extra
> >>> justification or else revert? .....
> >>>
> >>> On Wed, Jul 8, 2015 at 12:39 PM, asfgit <gi...@git.apache.org> wrote:
> >>> > Github user asfgit closed the pull request at:
> >>> >
> >>> >     https://github.com/apache/cloudstack/pull/567
> >>> >
> >>> >
> >>> > ---
> >>> > If your project is set up for it, you can reply to this email and
> have
> >>> your
> >>> > reply appear on GitHub as well. If your project does not have this
> >>> feature
> >>> > enabled and wishes so, or if the feature is enabled but not working,
> >>> please
> >>> > contact infrastructure at infrastructure@apache.org or file a JIRA
> >>> ticket
> >>> > with INFRA.
> >>> > ---
> >>>
> >>>
> >>>
> >>> --
> >>> Daan
> >>>
> >
> >
> >
> > --
> > Daan
>

Re: revert or not revert? (was: [GitHub] cloudstack pull request: CLOUDSTACK-8583 : fixing issue related to...)

Posted by David Nalley <da...@gnsa.us>.
More tests are good, on the other hand exceptions tend to beget
exceptions at tremendous speed.

On Wed, Jul 8, 2015 at 7:48 AM, Daan Hoogland <da...@gmail.com> wrote:
> That was never formalized, hence I put it as a shakespearian question.
> I did sugest it otherwise I would have just reverted. I don't like the
> exception but if the number of people working on integration is to low
> to have a good flow we could.
>
> On Wed, Jul 8, 2015 at 1:46 PM, Sanjeev N <sa...@apache.org> wrote:
>> I thought there is a limitation on LGTMs for integration tests?
>>
>> On Wed, Jul 8, 2015 at 5:11 PM, Daan Hoogland <da...@gmail.com>
>> wrote:
>>
>>> guys and dolss,
>>>
>>> this was closed with only 1 LGTM! we agreed not to submit with less
>>> then two ok reviews on master. So next steps? revert? some extra
>>> justification or else revert? .....
>>>
>>> On Wed, Jul 8, 2015 at 12:39 PM, asfgit <gi...@git.apache.org> wrote:
>>> > Github user asfgit closed the pull request at:
>>> >
>>> >     https://github.com/apache/cloudstack/pull/567
>>> >
>>> >
>>> > ---
>>> > If your project is set up for it, you can reply to this email and have
>>> your
>>> > reply appear on GitHub as well. If your project does not have this
>>> feature
>>> > enabled and wishes so, or if the feature is enabled but not working,
>>> please
>>> > contact infrastructure at infrastructure@apache.org or file a JIRA
>>> ticket
>>> > with INFRA.
>>> > ---
>>>
>>>
>>>
>>> --
>>> Daan
>>>
>
>
>
> --
> Daan

Re: revert or not revert? (was: [GitHub] cloudstack pull request: CLOUDSTACK-8583 : fixing issue related to...)

Posted by Daan Hoogland <da...@gmail.com>.
That was never formalized, hence I put it as a shakespearian question.
I did sugest it otherwise I would have just reverted. I don't like the
exception but if the number of people working on integration is to low
to have a good flow we could.

On Wed, Jul 8, 2015 at 1:46 PM, Sanjeev N <sa...@apache.org> wrote:
> I thought there is a limitation on LGTMs for integration tests?
>
> On Wed, Jul 8, 2015 at 5:11 PM, Daan Hoogland <da...@gmail.com>
> wrote:
>
>> guys and dolss,
>>
>> this was closed with only 1 LGTM! we agreed not to submit with less
>> then two ok reviews on master. So next steps? revert? some extra
>> justification or else revert? .....
>>
>> On Wed, Jul 8, 2015 at 12:39 PM, asfgit <gi...@git.apache.org> wrote:
>> > Github user asfgit closed the pull request at:
>> >
>> >     https://github.com/apache/cloudstack/pull/567
>> >
>> >
>> > ---
>> > If your project is set up for it, you can reply to this email and have
>> your
>> > reply appear on GitHub as well. If your project does not have this
>> feature
>> > enabled and wishes so, or if the feature is enabled but not working,
>> please
>> > contact infrastructure at infrastructure@apache.org or file a JIRA
>> ticket
>> > with INFRA.
>> > ---
>>
>>
>>
>> --
>> Daan
>>



-- 
Daan

Re: revert or not revert? (was: [GitHub] cloudstack pull request: CLOUDSTACK-8583 : fixing issue related to...)

Posted by Sanjeev N <sa...@apache.org>.
I thought there is a limitation on LGTMs for integration tests?

On Wed, Jul 8, 2015 at 5:11 PM, Daan Hoogland <da...@gmail.com>
wrote:

> guys and dolss,
>
> this was closed with only 1 LGTM! we agreed not to submit with less
> then two ok reviews on master. So next steps? revert? some extra
> justification or else revert? .....
>
> On Wed, Jul 8, 2015 at 12:39 PM, asfgit <gi...@git.apache.org> wrote:
> > Github user asfgit closed the pull request at:
> >
> >     https://github.com/apache/cloudstack/pull/567
> >
> >
> > ---
> > If your project is set up for it, you can reply to this email and have
> your
> > reply appear on GitHub as well. If your project does not have this
> feature
> > enabled and wishes so, or if the feature is enabled but not working,
> please
> > contact infrastructure at infrastructure@apache.org or file a JIRA
> ticket
> > with INFRA.
> > ---
>
>
>
> --
> Daan
>