Code review required for commits - formal Team vote.

[Ira Cooper]

On Sat, Oct 13, 2012 at 7:28 AM, Christian Ambach <ambi at samba.org> wrote:
> On 10/12/2012 07:48 PM, Jeremy Allison wrote:
>
>> --------------------------------------------------------
>> 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.
>>
>> Tools for code review to be decided amongst reviewers, but any
>> patch posted to samba-technical is an explicit request for review.
>>
>> Release branches work as they do today, though the + should be treated
>> as a sign-off in bugzilla. All release branch bug commits must contain
>> their bug number in them, so we can track back what happened.
>> --------------------------------------------------------
>
>
> In principle, I would vote +1 for this proposal.
>
> But I can also understand some of the reservations that others have,
> especially if it comes to patches to areas of the code that are not under
> regular development near to being un-maintained at all.
> It will be difficult to find a second reviewer for this.
>
> Other areas might be special VFS modules that are made for certain client
> applications (like catia, mediaharmony) or uncommon platforms (e.g.
> OpenAFS).
>
> So I'll put my vote on hold until the concerns have been resolved.

Well put sir.

I agree with your views, and share your concerns, and vote "I haven't
voted yet."

My concern follows, and may help some of yours and others:

I'm also concerned that patches will fall through the cracks.

There must be a way that we can know what reviews need to be done
easily, and monitor the situation.  I don't know how to do this, but I
think we should.  Just so we actually make sure the obscure patches
get reviewed.

I know at work, I have a website I goto, and it lists my reviews I
have to get to.  That's handy, but it is not the requirement.  A
command tool, a radio station, samba branded carrier pigeons, anything
will work.  It merely needs to exist.  (I believe this echoes some of
Matthieu's concerns.)  Given that I think this requirement can be
solved about 18,000 ways.  It is not a "show stopper."  But it is a
"release blocker" shall we say.

Thanks,

-Ira

PS: Jeremy, I respect you and your time too much to let you become the
device mentioned above, except as a stop-gap measure.