Open Stack

On 16 October 2013 09:54, Vishvananda Ishaya <vishvananda at gmail.com> wrote:
> Hi Everyone,
>
> I've been following this conversation and weighing the different sides. This is a tricky issue but I think it is important to decouple further and extend our circle of trust.
>
> When nova started it was very easy to do feature development. As it has matured the pace has slowed. This is expected and necessary, but we periodically must make decoupling decisions or we will become mired in overhead. We did this already with cinder and neutron, and we have discussed doing this with virt drivers in the past.
>
> We have a large number of people attempting to contribute to small sections of nova and getting frustrated with the process.  The perception of developers is much more important than the actual numbers here. If people are frustrated they are disincentivized to help and it hurts everyone. Suggesting that these contributors need to learn all of nova and help with the review queue is silly and makes us seem elitist. We should make it as easy as possible for new contributors to help.


So I agree that perception is a big and significant thing here, but...

I think the fundamental issue is review latency:
http://russellbryant.net/openstack-stats/nova-openreviews.html

Stats since the last revision without -1 or -2 (ignoring jenkins):

Average wait time: 6 days, 17 hours, 24 minutes
1rd quartile wait time: 0 days, 19 hours, 25 minutes
Median wait time: 3 days, 20 hours, 3 minutes
3rd quartile wait time: 7 days, 7 hours, 16 minutes

At the moment 25% of the time Nova reviews sit for over a week [btw we
probably want to not ignore Jenkins in that statistic, as I suspect
reviewers tend to ignore patches that Jenkins hated on].

Now, you may say 'hey, 7 days isn't terrible', but actually it is: if
a developer is producing (say) 2 patches a day, then after 7 calendar
days they may have as many as 10 patches in a vertical queue awaiting
review. Until the patch is reviewed for design-and-architectural
issues (ignoring style stuff which isn't disruptive), all the work
they are doing may need significant rework. Having to redo a week of
work is disruptive to the contributor.

The median wait time of 3 days would nearly halve the amount of rework
that can occur, which would be a significant benefit.

Nova has (over the last 30 days) -
http://russellbryant.net/openstack-stats/nova-reviewers-30.txt:
Total reviews: 3386
Total reviewers: 173

By my count 138 of the reviewers have done less than 20 reviews in
that 30 day period - thats less than one review a day. -
https://docs.google.com/spreadsheet/ccc?key=0AlLkXwa7a4bpdDNjd2gtTE1odjJRYjRVWjhhR2VKQVE&usp=sharing

So, 520 reviews from that 138 folk, but if they did 1 per weekday,
that would be 2760 reviews, bringing the total to 3386+2760-520=5626
reviews - or nearly *double* the review bandwidth.

Now, that won't get you more cores overnight, but that sustained
effort learning about the codebase will bring significant knowledge to
a wide set of folk - and thats what's needed to become core, and
increase the approval bandwidth in the team. I don't know exactly what
russell looks for in managing the set of -core, but surely having
enough knowledge is part of it.

Now, I've nothing against having reviewers specialise in a part of the
code base, and making that official - but I think it must be paired
with still requiring substantial knowledge of the projects code and
architecture: just because something is changing in e.g.
nova/virt/disk/api.py doesn't make it irrelevant to specific virt
drivers, and the right way to use something in the rest of the code
base is also relevant to virt drivers, and knowing *whats there to
reuse* is also important.

So, I guess my proposal is, make a restricted +2 category of reviewer:
 - social agreement to +2 only in enumerated areas
 - still need widespread knowledge of nova's code
 - best demonstrated by sustained regular reviews of other changes
 - but granted after a shorter incubation period
 - migrates to full in time
 - privilege and responsibility lost by the same criteria as all other reviewers

Of course, if this has been tried, fine - but AFAICT 'contribute
equally' hasn't been tried yet.

-Rob



-- 
Robert Collins <rbtcollins at hp.com>
Distinguished Technologist
HP Converged Cloud