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. <arma...@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 <ru...@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 <dh...@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 <mest...@mestery.com> >>> wrote: >>> >>>> On Thu, Jul 31, 2014 at 7:11 AM, Yuriy Taraday <yorik....@gmail.com> >>>> wrote: >>>> > On Wed, Jul 30, 2014 at 11:52 AM, Kyle Mestery <mest...@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@lists.openstack.org >>>> > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >>>> > >>>> >>>> _______________________________________________ >>>> OpenStack-dev mailing list >>>> OpenStack-dev@lists.openstack.org >>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >>>> >>> >>> >>> _______________________________________________ >>> OpenStack-dev mailing list >>> OpenStack-dev@lists.openstack.org >>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >>> >>> >> >> _______________________________________________ >> OpenStack-dev mailing list >> OpenStack-dev@lists.openstack.org >> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >> >> > > _______________________________________________ > OpenStack-dev mailing list > OpenStack-dev@lists.openstack.org > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev > >
_______________________________________________ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
