[OpenStack-Infra] [openstack-dev] [all][infra][tc][ptl] Scaling up code review process (subdir cores)

Salvatore Orlando sorlando at nicira.com
Tue Jun 2 22:14:22 UTC 2015


On 2 June 2015 at 23:59, Ian Cordasco <ian.cordasco at rackspace.com> wrote:

>
>
> On 6/2/15, 16:24, "Boris Pavlovic" <boris at pavlovic.me> wrote:
>
> >Hi stackers,
> >
> >
> >Issue
> >-------
> >
> >
> >Projects are becoming bigger and bigger overtime.
> >More and more people would like to contribute code and usually core
> >reviewers
> >team can't scale enough. It's very hard to find people that understand
> >full project and have enough time to do code reviews. As a result team is
> >very small under heavy load and many maintainers just get burned out.
> >
> >
> >We have to solve this issue to move forward.
> >
> >
> >
> >
> >Idea
> >------
> >
> >
> >Let's introduce subsystems cores.
> >
> >
> >Really it's hard to find cores that understand whole project, but it's
> >quite simple to find people that can maintain subsystems of project.
> >
> >
> >
> >
> >How To
> >-----------
> >
> >
> >Gerrit is not so simple as it looks and it has really neat features ;)
> >
> >
> >For example we can write own rules about who can put +2 and merge patch
> >based on changes files.
> >
> >
> >We can make special "subdirectory core" ACL group.
> >People from such ACL group will be able to merge changes that touch only
> >files from some specific subdirs.
> >
> >
> >As a result with proper organization of directories in project we can
> >scale up review process without losing quality.
> >
> >
> >
> >
> >Thoughts?
> >
> >
> >
> >
> >Best regards,
> >Boris Pavlovic
>
> I like this very much. I recall there was a session at the summit about
> this that Thierry and Kyle led.


Indeed, and Kyle has already transformed that into facts [1]


> If I recall correctly, the discussion
> mentioned that it wasn't (at this point in time) possible to use gerrit
> the way you describe it, but perhaps people were mistaken?
>

I recall that too, and I also recall fungi stating the same thing back in
Paris.
Gerrit doesn't really have a concept of subsystems, as far as I can
understand; in theory gerrit could be changed to support this, but that's
another discussion.
The networking community is currently adopting multiple repositories to
this aim. This has worked very well for 3rd party plugins, and quite well
for advanced services.
For the 'neutron' proper project, which is however large enough to identify
multiple subsystems in it, the lieutenant mode described in [1] will be
enforced with a bit of common sense - from what I gather. If you're a core
for subsystem X, nominated by its lieutenant, you're not supposed to +/-2
patches that only marginally affect your subsystem or do not affect it at
all.


>
> If we can do this exactly as you describe it, that would be awesome.

If
> there's a problem in limiting people to what files they can approve
> changes for, then an alteration might be that those people get +2 but not
> +W. This provides a signal to whomever has +W that the review is very much
> ready to be merged. Does that sound fair?
>

neutron-specs adopts this approach (all cores can +2 but only a handful can
+A).
I think it works, in the assumption of a lieutenant systems, but for
projects with a large patch turnaround might constitute a bottleneck,
especially when there are gate-breaking issues that need to be approved
ASAP.
Generally speaking, I believe having 2 ties of cores (those with +A rights
and those without) is an experiment worth doing. I don't think it creates
an "elite" among developers; on the other hand, it gives SMEs a chance to
have a greater impact.



> Cheers,
> Ian
>
>
Salvatore

[1]
http://git.openstack.org/cgit/openstack/neutron/tree/doc/source/policies/core-reviewers.rst


> __________________________________________________________________________
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-infra/attachments/20150603/c1e8bf1c/attachment-0001.html>


More information about the OpenStack-Infra mailing list