Code review required for commits - Discuss.

[Matthieu Patou]

On 10/11/2012 12:48 PM, Ira Cooper wrote:
> Jeremy and Simo brought this up in another topic, but it deserves its
> own thread really.
>
> Personally:
>
> I have a rate of "bit rot" I expect on Illumos/Solaris.  It's usually
> 1-2 build breaks, and a few minor issues if I walk away for 6 months.
> Yes... I have expectations on just how broken things get.  Sometimes
> we all exceed them.  Sometimes not.  But the rule of thumb: "Master
> won't build 100% right if I haven't touched it in 6 months." is right.
>
> Also, in most serious work I've done, I've always had reviewers before
> committing my code, provided I'm not the only one working on it.  It
> is just standard procedure.
>
> So consider this my +1, to their ideas, and giving them a "rule".
>
> Actual formal suggestion:
>
> No team member commits their own code.  All code will be "signed off"
> by two team members, as a team member you may sign off your own code.
> The "non-author" team member will be responsible for pushing the code.
>   If there are two they can agree among themselves. ;)
To my french mind the wording seems a bit confusing, are you saying that:
1) every author could (should?) add their own sign-off
2) in total 2 sign-off are need, which means at least 1 non-author 
review (but can be two if the author decide not to sign-off it's patch)
> Release branches work as they do today, though the + should be treated
> as a sign-off in bugzilla.
Why-not but should it be automatic ?
> All bug commits should now contain their
> bug number in them, so we can track back what happened.
Agreed.

In general I'm mostly ok with this proposal but we have to aware that 
will increase the workload on some of us.
Also I want to have precision on how to handle the failure in autobuild 
and I'd like to see a kind of watchdog so that patches can't wait more 
that xx days for review.

Finally as we did so far we can try this rule on a voluntary basis (it 
was the case for autobuild too) and see how it flies for 1 or 2 months.

Matthieu.

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