Open Stack

On Fri, Aug 29, 2014 at 11:07:33AM +0200, Thierry Carrez wrote:
> Joe Gordon wrote:
> > On Thu, Aug 28, 2014 at 2:43 PM, Alan Kavanagh
> > <alan.kavanagh at ericsson.com <mailto:alan.kavanagh at ericsson.com>> wrote:
> > 
> >>     I share Donald's points here, I believe what would help is to
> >>     clearly describe in the Wiki the process and workflow for the BP
> >>     approval process and build in this process how to deal with
> >>     discrepancies/disagreements and build timeframes for each stage and
> >>     process of appeal etc.
> >>     The current process would benefit from some fine tuning and helping
> >>     to build safe guards and time limits/deadlines so folks can expect
> >>     responses within a reasonable time and not be left waiting in the cold.
> > 
> > This is a resource problem, the nova team simply does not have enough
> > people doing enough reviews to make this possible. 
> 
> I think Nova lacks core reviewers more than it lacks reviewers, though.
> Just looking at the ratio of core developers vs. patchsets proposed,
> it's pretty clear that the core team is too small:
> 
> Nova: 750 patchsets/month for 21 core = 36
> Heat: 230/14 = 16
> Swift: 50/16 = 3
> 
> Neutron has the same issue (550/14 = 39). I think above 20, you have a
> dysfunctional setup. No amount of process, spec, or runway will solve
> that fundamental issue.
> 
> The problem is, you can't just add core reviewers, they have to actually
> understand enough of the code base to be trusted with that +2 power. All
> potential candidates are probably already in. In Nova, the code base is
> so big it's difficult to find people that know enough of it. In Neutron,
> the contributors are often focused on subsections of the code base so
> they are not really interested in learning enough of the rest. That
> makes the pool of core candidates quite dry.
> 
> I fear the only solution is smaller groups being experts on smaller
> codebases. There is less to review, and more candidates that are likely
> to be experts in this limited area.
> 
> Applied to Nova, that means modularization -- having strong internal
> interfaces and trusting subteams to +2 the code they are experts on.
> Maybe VMWare driver people should just +2 VMware-related code. We've had
> that discussion before, and I know there is a dangerous potential
> quality slope there -- I just fail to see any other solution to bring
> that 750/21=36 figure down to a bearable level, before we burn out all
> of the Nova core team.

I broadly agree - I think that unless Nova moves more towards something
that is closer to the Linux style subsystem maintainer model we are 
doomed. I know in Linux, the maintainers actually use separate git trees,
and that isn't what I mean - I think using a single git tree is still
desirable (at least for now). What I mean is that we should place more
trust on the opinion of the people who are experts for a particular
area of code. Let those experts take on a greater burden of the code
review so core team can put more focus on actual merge approval.

I know some of the core team try to do this implicitly - eg we know who
some of the main people involved in hyperv or vmware are, so will tend
to treat their +1 as an effective +2 from the POV of their driver code,
but our rules still require two actual +2s from core, so it doesn't
entirely help us right now. I think we need to do some work in tooling
to make this more of an explicit process though.

The problem is that gerrit does not allow us to say person X has +2 for
code that touches directory path /foo/bar. The +2 is global to the entire
repository. We could try to deal with this problem outside of gerrit
though. As a starting point, each virt driver (or major functional area
of nova codebase) should have an explicit list of people who are considered
to be the "core team" for that area of code.  From such a list, tools like
gerrymander (or anything else that can query gerrit), could see when a
person in those lists +1'd a change touching their area of responsibility
and change that to be presented as a "+1.5".

This would make it very explicit to the reviewers that they should consider
the change to be (almost) equivalent to a +2.  We could potentially then
relax the rule of

     "+A requires two +2s"

to be 

     "+A requires (two +2s) or (one +2 and one +1.5)"

I think this would significantly improve our review throughput. I think
it could also help us get people to gain greater responsibility. The
jump from regular contributor to core team member is quite a high bar.
If we had the intermediate step of subsystem team member that would ease
the progression. It would also give the subsystem teams a greater sense
of engagement & value in the nova community

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|