Require TWO team members review for externally written patches?

[Michael Adam]

On 2013-06-03 at 08:25 +1000, Andrew Bartlett wrote:
> On Mon, 2013-06-03 at 00:16 +0200, Michael Adam wrote:
> > 
> > Well, Andrew already pushed the patches. Now they are not only
> > missing the signoff but also a second review by a team member.  :-)
> 
> While I've seen Jeremy doing that, I never understood that to be the
> process,

See what we have written down as a summary of our discussions
last year:

https://wiki.samba.org/index.php/CodeReview

There was always the proposed requirement to have two team
members review, with the special case tha the author's signoff
would count as review if the author happens to be a team member.

> and if we must discuss that again I would oppose it, as it only
> adds friction to the typically small and useful patches that our
> external contributors bring. 
> 
> The purpose of the review process is to ensure someone else, other than
> the author, thinks it's OK.  That reviewer may want extra review (I
> asked that a patch go past kai or amitay as well recently, despite being
> a one-liner), but I see NO value in *requiring* the involvement in two
> team members.  It just makes extra work for value.

Here we are again at the fundamental question what is a small
patch. What is a harmless and/or useful patch? It is not possible
to draw a line here. So we need a clean and simple rule that
applies always. If the patch is so simple and harmless than it
will be very easy to review and get additional review. The hard
part is the review of the complex patches.

Cheers - Michael