Open Stack

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, 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.

> 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 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
>