Code review required for commits - Discuss.

[Matthieu Patou]

>>> As far as I am concerned 1 signoff (author) and 1 ack should be
>>> sufficient, independently of who is the author.
>>> However it would be really desirable that who acks is the maintainer.
>> Why not at the opposite encouraging review by someone not necessarily
>> familiar with area of the code ? so that we tend to encourage involvement in
>> parts where we usually don't work ?
>>
>> As for sensitive part people concerned about the patches (ie. maintainer(s))
>> could always pop-up on the list and tell that they want to do a review as
>> well and do it within 7 days.
> Matthieu,
>
> You got it 1/2 right, I agree people should do what they can to expand
> as they can.  But the whole "maintainers" system is the 100% anthesis
> of that.
>
> No kings, no popes.  If you are respected, people will defer to you
> naturally.  Order should come from the chaos naturally, not a file.
Sure but as I said we might want to promote implication of team members 
in area that they usually don't touch (ie. fileserver for me, AD DC 
related stuff for you) but still you want to have a safety net so that I 
can't give a blessing on modification on libtevent (for instance).
> "MAINTAINERS" should come out as a cultural convention.  People defer
> to those more knowledgable.
>
> As far as the time duration.  I stand by 3 days.  If you can't get to
> it in 3 days.. you aren't getting to it.
I think you didn't get me right here, so first I should say that the rule:
"I post on the list and if nobody react withing 3 business days -> to 
master tree" is very ok for me, I like it!

My point was more the following:
1) I post a patch for winbindd on monday
2) On tuesday you say I'm gonna review it
3) On tuesday afternoon micheal adams says: "I also want to review it"
4) On wednesday you say I acknowledge the patch

Then starting from now Micheal has to next Sunday to review my patch or 
you will push it (as you acknowledged it). As I said this should allow 
people to say "I really want to review this patch set" but without 
blocking something for too long.

Note that this is maybe a bit complicated though, if anybody has a 
better idea to gently force members to review code not necessarily in 
their core expertise.

Matthieu.

-- 
Matthieu Patou
Samba Team
http://samba.org