Code review required for commits - formal Team vote.

[Jeremy Allison]

On Wed, Oct 17, 2012 at 01:00:58AM +0200, Michael Adam wrote:
> So, since we are discussing anyways, and the discussion takes
> a lot longer and gets a lot more heated than I had anticipated,
> I think we should take some momentum out of it and not force
> things.
> 
> SUGGESTION:
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 1. I suggest that we defer a team vote on a commited trial period
>    or even a policy change for some time, at least a couple of weeks
>    until we have thought and discussed things through thoroughly
>    and without hastening, maybe until after 4.0 is released.

I really don't want to wait that long. IMHO I think we
do need to resolve this as soon as possible. "Waiting for
4.0" means next year, which I think is too late.

> 2. In the meanwhile, we encourage team members to do reviews
>    more often on a voluntary basis, which means both asking for
>    review before pushing and giving review for others.

I've certainly started to lead by example here, and
will continue to do so.

> 3. We also encourage to have the reviewer push.

Again, I will adhere to this.

> 4. We can also try any mechanisms/tools on a voluntary basis.
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This would be really helpful. Do you have any in particular
you'd like to set up ?

> This allows us to concentrate on shipping 4.0 which is the most
> important goal currently! The worst we can do now is delay or
> spoil the 4.0 effort by working ourselves into the ground over
> heated policy discussions.

Shipping 4.0.0 proceeds in parallel, based on bug reports.
I think we can do both (I certainly can :-).

> And it allows us on a completely voluntary basis to gain more
> experience and confidence in the reviewing business and possibly
> get an idea of the load and mechanisms.

The problem with "completely voluntary" is that master will
keep getting screwed up with poorly reviewed patches.

Note I'm as guilty as anyone here of this, but I'm really,
*really* trying to fix it.

As Christian and Ira (and many others) have noted, breaking the
master build should no longer be acceptible.

> This will certainly move towards mandatory reviews much slower
> than the hard trial period, but it is more respectful for the
> folks that have serious concerns. I think that we will still make
> progress, based on the following observations:
> 
> - I think we can safely assume that we all want to sustain and
>   improve the quality of our commits and would consider changing
>   our mode of operation on the master branch if suitable.
> 
> - I got from mails and discussions that nobody actually
>   thinks that doing reviews is a bad thing!

Well this is progress !

> The concerns we should address are these:
> 
> - How will we provide the ressources to review 6000 patches / year?
>   (or will it imply a drop in productivity?)

6000 patches, but how many *patchsets* ? Patches don't get
reviewed, patchsets do. There's an important distinction
here.

> - Do we need a tracking tool for reviews so patches don't get lost?
>   (this is roughly a.k.a how do I get a reviewer for my patch?)

Right now I'm using my inbox. What would you suggest as
a different tool ? At work we use gerrit, but Simo has
some valid (Java cough, cough :-) reasons to avoid that :-).

> - Can we find *two* team reviewers even for the most exotic areas
>   in code ore do we need to establish more flexible rules?

Yes, we can certainly do this. In fact we *must*. No area
of the code should be understood by just one person, that
is a terrible "bus-factor" to use a Linux kernel term :-).

> I hope that even a voluntary period (as has been suggested by
> others) will lead us to some insight about theses issues in the
> next couple of weeks and that we can move forward with voting
> about something more concrete and with a better feeling attached
> to it after 4.0 is out.

That's too much like waiting for Godot to me :-). I think we
can proceed without waiting for 4.0.0.

> PS: Jeremy, I apologize for back-pedalling here. I still think
> we should do it, but not by all means. This is the only way
> I see how we can continue in a reasonable and respectful way.

We need to address this issue in a timely fashion. What time
limit are you suggesting on discussions ?

What concerns me about "agreeing to more discussions" is 
this is an open-ended way to indefinitely postpone the
issue. I don't want that to happen.

Jeremy.