Open Stack

Hi Armando:

>  If a core-reviewer puts a -2, there must be a good reason for it

I agree. The problem is that after the initial issue identified in the
initial -2 review has been fixed, and the patch updated, it (sometimes)
happens that we can not get the original reviewer to re-review that update
for weeks - creating the type of issues identified in this thread.

I would agree that if this was a one-off scenarios, we should handling this
as a specific case as you suggest. Unfortunately, this is not a one-off
instance, and hence my request for clearer guidelines from PTL for such
cases.

Regards,
Mandeep



On Thu, Jul 31, 2014 at 3:54 PM, Armando M. <armamig at gmail.com> wrote:

> It is not my intention debating, pointing fingers and finding culprits,
> these issues can be addressed in some other context.
>
> I am gonna say three things:
>
> 1) If a core-reviewer puts a -2, there must be a good reason for it. If
> other reviewers blindly move on as some people seem to imply here, then
> those reviewers should probably not review the code at all! My policy is to
> review all the code I am interested in/I can, regardless of the score. My
> -1 may be someone's +1 (or vice versa), so 'trusting' someone else's vote
> is the wrong way to go about this.
>
> 2) If we all feel that this feature is important (which I am not sure it
> was being marked as 'low' in oslo, not sure how it was tracked in Neutron),
> there is the weekly IRC Neutron meeting to raise awareness, since all cores
> participate; to the best of my knowledge we never spoke (or barely) of the
> rootwrap work.
>
> 3) If people do want this work in Juno (Carl being one of them), we can
> figure out how to make one final push, and assess potential regression. We
> 'rushed' other features late in cycle in the past (like nova/neutron event
> notifications) and if we keep this disabled by default in Juno, I don't
> think it's really that risky. I can work with Carl to give the patches some
> more love.
>
> Armando
>
>
>
> On 31 July 2014 15:40, Rudra Rugge <rudra at contrailsystems.com> wrote:
>
>> Hi Kyle,
>>
>> I also agree with Mandeep's suggestion of putting a time frame on the
>> lingering "-2" if the addressed concerns have been taken care of. In my
>> experience also a sticky -2 detracts other reviewers from reviewing an
>> updated patch.
>>
>> Either a time-frame or a possible override by PTL (move to -1) would help
>> make progress on the review.
>>
>> Regards,
>> Rudra
>>
>>
>> On Thu, Jul 31, 2014 at 2:29 PM, Mandeep Dhami <dhami at noironetworks.com>
>> wrote:
>>
>>> Hi Kyle:
>>>
>>> As -2 is sticky, and as there exists a possibility that the original
>>> core might not get time to get back to re-reviewing his, do you think that
>>> there should be clearer guidelines on it's usage (to avoid what you
>>> identified as "dropping of the balls")?
>>>
>>> Salvatore had a good guidance in a related thread [0], do you agree with
>>> something like that?
>>>
>>>
>>> I try to avoid -2s as much as possible. I put a -2 only when I reckon your
>>> patch should never be merged because it'll make the software unstable or
>>> tries to solve a problem that does not exist. -2s stick across patches and
>>> tend to put off other reviewers.
>>>
>>> [0]
>>> http://lists.openstack.org/pipermail/openstack-dev/2014-July/041339.html
>>>
>>>
>>> Or do you think that 3-5 days after an update that addresses the issues
>>> identified in the original -2, we should automatically remove that -2? If
>>> this does not happen often, this process does not have to be automated,
>>> just an "exception" that the PTL can exercise to address issues where the
>>> original reason for -2 has been addressed and nothing new has been
>>> identified?
>>>
>>>
>>>
>>> On Thu, Jul 31, 2014 at 11:25 AM, Kyle Mestery <mestery at mestery.com>
>>> wrote:
>>>
>>>> On Thu, Jul 31, 2014 at 7:11 AM, Yuriy Taraday <yorik.sar at gmail.com>
>>>> wrote:
>>>> > On Wed, Jul 30, 2014 at 11:52 AM, Kyle Mestery <mestery at mestery.com>
>>>> wrote:
>>>> >> and even less
>>>> >> possibly rootwrap [3] if the security implications can be worked out.
>>>> >
>>>> > Can you please provide some input on those security implications that
>>>> are
>>>> > not worked out yet?
>>>> > I'm really surprised to see such comments in some ML thread not
>>>> directly
>>>> > related to the BP. Why is my spec blocked? Neither spec [1] nor code
>>>> (which
>>>> > is available for a really long time now [2] [3]) can get enough
>>>> reviewers'
>>>> > attention because of those groundless -2's. Should I abandon these
>>>> change
>>>> > requests and file new ones to get some eyes on my code and proposals?
>>>> It's
>>>> > just getting ridiculous. Let's take a look at timeline, shall we?
>>>> >
>>>> I share your concerns here as well, and I'm sorry you've had a bad
>>>> experience working with the community here.
>>>>
>>>> > Mar, 25 - first version of the first part of Neutron code is
>>>> published at
>>>> > [2]
>>>> > Mar, 28 - first reviewers come and it gets -1'd by Mark because of
>>>> lack of
>>>> > BP (thankful it wasn't -2 yet, so reviews continued)
>>>> > Apr, 1 - Both Oslo [5] and Neturon [6] BPs are created;
>>>> > Apr, 2 - first version of the second part of Neutron code is
>>>> published at
>>>> > [3];
>>>> > May, 16 - first version of Neutron spec is published at [1];
>>>> > May, 19 - Neutron spec gets frozen by Mark's -2 (because Oslo BP is
>>>> not
>>>> > approved yet);
>>>> > May, 21 - first part of Neutron code [2] is found generally OK by
>>>> reviewers;
>>>> > May, 21 - first version of Oslo spec is published at [4];
>>>> > May, 29 - a version of the second part of Neutron code [3] is
>>>> published that
>>>> > later raises only minor comments by reviewers;
>>>> > Jun, 5 - both parts of Neutron code [2] [3] get frozen by -2 from Mark
>>>> > because BP isn't approved yet;
>>>> > Jun, 23 - Oslo spec [4] is mostly ironed out;
>>>> > Jul, 8 - Oslo spec [4] is merged, Neutron spec immediately gets +1
>>>> and +2;
>>>> > Jul, 20 - SAD kicks in, no comments from Mark or anyone on blocked
>>>> change
>>>> > requests;
>>>> > Jul, 24 - in response to Kyle's suggestion I'm filing SAD exception;
>>>> > Jul, 31 - I'm getting final "decision" as follows: "Your BP will
>>>> extremely
>>>> > unlikely get to Juno".
>>>> >
>>>> > Do you see what I see? Code and spec is mostly finished in May (!)
>>>> where the
>>>> > "mostly" part is lack of reviewers because of that Mark's -2. And 1
>>>> month
>>>> > later when all bureaucratic reasons fall off nothing happens. Don't
>>>> think I
>>>> > didn't try to approach Mark. Don't think I didn't approach Kyle on
>>>> this
>>>> > issue. Because I did. But nothing happens and another month passes by
>>>> and I
>>>> > get "You know, may be later" general response. Noone (but those who
>>>> knew
>>>> > about it originally) even looks at my code for 2 months already
>>>> because Mark
>>>> > doesn't think (I hope he did think) he should lift -2 and I'm getting
>>>> "why
>>>> > not wait another 3 months?"
>>>> >
>>>> > What the hell is that? You don't want to land features that doesn't
>>>> have
>>>> > code 2 weeks before Juno-3, I get that. My code has almost finished
>>>> code by
>>>> > 3.5 months before that! And you're considering to throw it to Kilo
>>>> because
>>>> > of some mystical issues that must've been covered in Oslo spec [4]
>>>> and if
>>>> > you like it they can be covered in Neutron spec [1] but you have to
>>>> let
>>>> > reviewers see it!
>>>> >
>>>> > I don't think that Mark's actions (lack of them, actually) are what's
>>>> > expected from core reviewer. No reaction to requests from developer
>>>> whose
>>>> > code got frozen by his -2. No reaction (at least no visible one) to
>>>> PTL's
>>>> > requests (and Kyle assured me he made those). Should we consider Mark
>>>> > uncontrollable and unreachable? Why does he have -2 right in the
>>>> first place
>>>> > then? So how should I overcome his inaction? I can recreate new change
>>>> > requests and hope he won't just -2 them with no comment at all. But
>>>> that
>>>> > would be just a sign of total failure of our shiny bureaucracy.
>>>> >
>>>> I have reached out a few times to Mark, and I'm not going to put words
>>>> in his mouth here, but what I can say is that the Neutron Core team
>>>> tries it's best to read all BPs and code which are submitted. In this
>>>> particular case, there was some dropping of the balls in how we
>>>> handled this. Carl has reached out to me a few times on this, and I've
>>>> reached out to Mark as well to remove the -2 here. Sometimes, even
>>>> with best intentions, things go awry.
>>>>
>>>> To move forward, there is interest in getting this feature upstream,
>>>> maybe even in Juno. But given some concerns I've heard from Mark and
>>>> now Thierry, maybe this does make sense to move to Kilo. I'll wait for
>>>> Mark to reply on this thread and chime in here, as well as Thierry if
>>>> he has more to say. Outside that, if Carl is willing to shepherd this
>>>> and we can get Mark to reply, it's still possible we can get this into
>>>> Juno.
>>>>
>>>> Thanks,
>>>> Kyle
>>>>
>>>> > [1] https://review.openstack.org/93889 - Neutron spec
>>>> > [2] https://review.openstack.org/82787 - first part of Neutron code
>>>> > [3] https://review.openstack.org/84667 - second part of Neutron code
>>>> > [4] https://review.openstack.org/94613 - Oslo spec
>>>> > [5] https://blueprints.launchpad.net/oslo/+spec/rootwrap-daemon-mode
>>>> > [6]
>>>> https://blueprints.launchpad.net/neutron/+spec/rootwrap-daemon-mode
>>>> >
>>>> > --
>>>> >
>>>> > Kind regards, Yuriy.
>>>> >
>>>> > _______________________________________________
>>>> > OpenStack-dev mailing list
>>>> > OpenStack-dev at lists.openstack.org
>>>> > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>>> >
>>>>
>>>> _______________________________________________
>>>> OpenStack-dev mailing list
>>>> OpenStack-dev at lists.openstack.org
>>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>>>
>>>
>>>
>>> _______________________________________________
>>> OpenStack-dev mailing list
>>> OpenStack-dev at lists.openstack.org
>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>>
>>>
>>
>> _______________________________________________
>> OpenStack-dev mailing list
>> OpenStack-dev at lists.openstack.org
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>
>>
>
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20140801/e97eeaf9/attachment.html>