Open Stack

(see comments inline)

----- Original Message -----
> From: "Jay Pipes" <jaypipes at gmail.com>
> To: openstack-dev at lists.openstack.org
> Sent: Thursday, September 4, 2014 2:34:28 PM
> Subject: Re: [openstack-dev] [Nova] Feature Freeze Exception process for Juno
> 
> On 09/03/2014 11:57 AM, Solly Ross wrote:
> >> I will follow up with a more detailed email about what I believe we are
> >> missing, once the FF settles and I have applied some soothing creme to
> >> my burnout wounds, but currently my sentiment is:
> >>
> >> Contributing features to Nova nowadays SUCKS!!1 (even as a core
> >> reviewer) We _have_ to change that!
> >
> > I think this is *very* important.
> >
> > <rant>
> > For instance, I have/had two patch series
> > up. One is of length 2 and is relatively small.  It's basically sitting
> > there
> > with one "+2" on each patch.  I will now most likely have to apply for a
> > FFE
> > to get it merged, not because there's more changes to be made before it can
> > get merged
> > (there was one small nit posted yesterday) or because it's a huge patch
> > that needs a lot
> > of time to review, but because it just took a while to get reviewed by
> > cores,
> > and still only appears to have been looked at by one core.
> >
> > For the other patch series (which is admittedly much bigger), it was hard
> > just to
> > get reviews (and it was something where I actually *really* wanted several
> > opinions,
> > because the patch series touched a couple of things in a very significant
> > way).
> >
> > Now, this is not my first contribution to OpenStack, or to Nova, for that
> > matter.  I
> > know things don't always get in.  It's frustrating, however, when it seems
> > like the
> > reason something didn't get in wasn't because it was fundamentally flawed,
> > but instead
> > because it didn't get reviews until it was too late to actually take that
> > feedback into
> > account, or because it just didn't get much attention review-wise at all.
> > If I were a
> > new contributor to Nova who had successfully gotten a major blueprint
> > approved and
> > the implemented, only to see it get rejected like this, I might get turned
> > off of Nova,
> > and go to work on one of the other OpenStack projects that seemed to move a
> > bit faster.
> > </rant>
> 
> I see that you have done 7 reviews in the past 90 days. Doing reviews on
> other people's code goes a long way towards getting some "core karma".

So, I've tried to get in to reviewing, but my experience tends to be that,
even with tools, I spend most of my time trying to find something to review.
I've found a few tags that I feel comfortable reviewing (because I know the
code paths well enough to give good feedback and not just nits).  However,
I often find myself on a thread where a couple of cores have already commented,
in which case I likely have very little to say.  This is not particularly an
excuse, just an explanation.

The other thing, of course, is that I get/got zoned in writing a large patch
series, and didn't stop much to review.  For me at least,
I would love to be able to put my name into some sort of pool, and said
"ping me if you need a review of X subjects".  If someone actually asked me for a
review, I would gladly put down my work for a bit and do a review, but I need to get
better and stopping on my own and proactively seeking out reviews.

Anyway, I think it would be useful to have some sort of page where people
could say "I'm an SME in X, ask me for reviews" and then patch submitters could go
and say, "oh, I need an someone to review my patch about storage backends, let me
ask sross".

> 
> > So, it's silly to rant without actually providing any ideas on how to fix
> > it.
> > One suggestion would be, for each approved blueprint, to have one or two
> > cores
> > explicitly marked as being responsible for providing at least some feedback
> > on
> > that patch.  This proposal has issues, since we have a lot of blueprints
> > and only
> > twenty cores, who also have their own stuff to work on.  However, I think
> > the
> > general idea of having "guaranteed" reviewers is not unsound by itself.
> > Perhaps
> > we should have a loose tier of reviewers between "core" and "everybody
> > else".
> > These reviewers would be known good reviewers who would follow the
> > implementation
> > of particular blueprints if a core did not have the time.  Then, when those
> > reviewers
> > gave the "+1" to all the patches in a series, they could ping a core, who
> > could feel
> > more comfortable giving a "+2" without doing a deep inspection of the code.
> 
> This is exactly the reason I believe there are so many places in Nova
> that we have such an astonishing amount of technical debt. Core
> reviewers are so swamped with the review load that they see +1s from
> folks that they perceive to be SMEs on certain parts of the code and
> give a less-than-needed-quality review of the patch and end up +2'ing it
> too early.
> 
> It is *precisely* the holistic understanding of multiple parts of the
> Nova codebase that is the critical piece that having 2 core reviewers
> sign-off on the patch. Cores are supposed to have a good grasp of
> multiple parts of the Nova subsystems and be able to tell (sometimes
> instinctively, sometimes explicitly) when a patch introduces behaviour
> that will incur more of the aforementioned technical debt. It is for
> this reason I strongly oppose any loosening of the 2 +2s system.

Ah, but here's the thing: currently, we don't have nearly enough cores
to maintain our current system.  I think @danpb's proposal to split out
the virt drivers would help alleviate this, since what is now a
VMWare SME could become a VMWare core more easily.

> 
> > That's just one suggestion, though.  Whatever the solution may be, this is
> > a
> > problem that we need to fix.  While I enjoyed going through the blueprint
> > process
> > this cycle (not sarcastic -- I actually enjoyed the whole "structured
> > feedback" thing),
> > the follow up to that was not the most pleasant.
> >
> > One final note: the specs referenced above didn't get approved until Spec
> > Freeze, which
> > seemed to leave me with less time to implement things.  In fact, it seemed
> > that a lot
> > of specs didn't get approved until spec freeze.  Perhaps if we had more
> > staggered
> > approval of specs, we'd have more staggered submission of patches, and thus
> > less of a
> > sudden influx of patches in the couple weeks before feature proposal
> > freeze.
> 
> I actually think the last paragraph above contains a good suggestion.
> This is, I think, similar to the "slots" approach that Joe Gordon and a
> couple other folks were championing at the mid-cycle and I think has
> quite a bit of merit. We need to find a way of:
> 
>   * focusing reviewers more effectively
>   * saying "no" more often and with more specific feedback
>   * being more structured in the way we communicate what is being
> actively reviewed and what isn't

I think one of the issues is that Gerrit doesn't seem to have a good
way to say "show me some patches that have low review velocity".  If
a patch submitter could manually say "looking for reviewer" (as opposed
to just having reviewers try to figure out if a patch needs reviewers)
and reviewers could easily filter on that, it might lower the bar for
"normal" contributors to start reviewing more.

> 
> I think the slots idea at least moves us in the proper direction
> regarding the above three things.
> 
> Best,
> -jay
> 
> > Best Regards,
> > Solly Ross
> >
> > ----- Original Message -----
> >> From: "Nikola Đipanov" <ndipanov at redhat.com>
> >> To: openstack-dev at lists.openstack.org
> >> Sent: Wednesday, September 3, 2014 5:50:09 AM
> >> Subject: Re: [openstack-dev] [Nova] Feature Freeze Exception process for
> >> Juno
> >>
> >> On 09/02/2014 09:23 PM, Michael Still wrote:
> >>> On Tue, Sep 2, 2014 at 1:40 PM, Nikola Đipanov <ndipanov at redhat.com>
> >>> wrote:
> >>>> On 09/02/2014 08:16 PM, Michael Still wrote:
> >>>>> Hi.
> >>>>>
> >>>>> We're soon to hit feature freeze, as discussed in Thierry's recent
> >>>>> email. I'd like to outline the process for requesting a freeze
> >>>>> exception:
> >>>>>
> >>>>>      * your code must already be up for review
> >>>>>      * your blueprint must have an approved spec
> >>>>>      * you need three (3) sponsoring cores for an exception to be
> >>>>>      granted
> >>>>
> >>>> Can core reviewers who have features up for review have this number
> >>>> lowered to two (2) sponsoring cores, as they in reality then need four
> >>>> (4) cores (since they themselves are one (1) core but cannot really
> >>>> vote) making it an order of magnitude more difficult for them to hit
> >>>> this checkbox?
> >>>
> >>> That's a lot of numbers in that there paragraph.
> >>>
> >>> Let me re-phrase your question... Can a core sponsor an exception they
> >>> themselves propose? I don't have a problem with someone doing that,
> >>> but you need to remember that does reduce the number of people who
> >>> have agreed to review the code for that exception.
> >>>
> >>
> >> Michael has correctly picked up on a hint of snark in my email, so let
> >> me explain where I was going with that:
> >>
> >> The reason many features including my own may not make the FF is not
> >> because there was not enough buy in from the core team (let's be
> >> completely honest - I have 3+ other core members working for the same
> >> company that are by nature of things easier to convince), but because of
> >> any of the following:
> >>
> >> * Crippling technical debt in some of the key parts of the code
> >> * that we have not been acknowledging as such for a long time
> >> * which leads to proposed code being arbitrarily delayed once it makes
> >> the glaring flaws in the underlying infra apparent
> >> * and that specs process has been completely and utterly useless in
> >> helping uncover (not that process itself is useless, it is very useful
> >> for other things)
> >>
> >> I am almost positive we can turn this rather dire situation around
> >> easily in a matter of months, but we need to start doing it! It will not
> >> happen through pinning arbitrary numbers to arbitrary processes.
> >>
> >> I will follow up with a more detailed email about what I believe we are
> >> missing, once the FF settles and I have applied some soothing creme to
> >> my burnout wounds, but currently my sentiment is:
> >>
> >> Contributing features to Nova nowadays SUCKS!!1 (even as a core
> >> reviewer) We _have_ to change that!
> >>
> >> N.
> >>
> >>> Michael
> >>>
> >>>>>      * exceptions must be granted before midnight, Friday this week
> >>>>> (September 5) UTC
> >>>>>      * the exception is valid until midnight Friday next week
> >>>>> (September 12) UTC when all exceptions expire
> >>>>>
> >>>>> For reference, our rc1 drops on approximately 25 September, so the
> >>>>> exception period needs to be short to maximise stabilization time.
> >>>>>
> >>>>> John Garbutt and I will both be granting exceptions, to maximise our
> >>>>> timezone coverage. We will grant exceptions as they come in and gather
> >>>>> the required number of cores, although I have also carved some time
> >>>>> out in the nova IRC meeting this week for people to discuss specific
> >>>>> exception requests.
> >>>>>
> >>>>> Michael
> >>>>>
> >>>>
> >>>>
> >>>> _______________________________________________
> >>>> 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
>