Code review required for commits - Discuss.

[Andrew Bartlett]

On Thu, 2012-10-11 at 15:48 -0400, Ira Cooper wrote:

> Actual formal suggestion:
> 
> No team member commits their own code.  

I really think this is a bad idea, and a good way to create a large
backlog for those of us who are fortunate enough to live in places that
are (allegedly!) currently enjoying a warm spring afternoon.

On a serious note, and taking note of your proposal to ensure that
changes can go in if no review is available, I think this specific rule,
applied unwaveringly across the tree creates significant disruption and
work without clear added value.

That is, given the vagaries of autobuild, it is not reasonable for the
reviewer and author to have to go back and forth, potentially across
multiple time zones and days, while getting a patch into the tree.  It
also dramatically increases the workload on the reviewer, who has not
only to inspect the patches, but also to find the time in their day for
what can be a multi-hour challenge to get a series of changes past our
testsuite.

I do appreciate review on my code, and you will have noticed that most
of my changes go past this list for review, particularly if they impact
on the source3 code in any substantive way.  Indeed, it is ironic that
the patch that seems to have sparked this crisis was positively reviewed
by the appropriate maintainer. 

If there are others interested in the work I'm doing (and we should have
those in at least the integration work), I'm happy to continue posting
the substantive patches for comment.  

Finally, on this proposal, I would like to have it clearly stated what
exactly (beyond that Google and others have the resources to do this)
the exact proposal is expected to achieve, and consider if this change
would really have addressed whatever failure is alleged to have
occurred. 

Thanks,

Andrew Bartlett

-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org