Code review required for commits - formal Team vote.

[Michael Adam]

On 2012-10-15 at 15:11 +1100, Andrew Bartlett wrote:
> On Sun, 2012-10-14 at 20:48 -0400, simo wrote:
> > On Mon, 2012-10-15 at 08:38 +1100, Andrew Bartlett wrote:
> > > On Sat, 2012-10-13 at 11:41 -0400, simo wrote:
> 
> > > Simo,
> > > 
> > > I'm confused by what you are saying you have done.  I don't think you
> > > are saying that you have not pushed any of your own patches in the past
> > > 2-3 years, but I just want to check.
> > 
> > No, what I did is that I obtained review for my patches, whether I push
> > them or not would make basically no difference, so that rule doesn't
> > worry me. I am confident my samba team mates can push as well as review.
> > 
> > I am also not welded on non-author push. If you have a clear acked-by
> > line sent to you for a specific patch, IMO you can push it yourself.
> > However I see value in reviewer-push as that assures the reviewer
> > actually applied the patch and didn't just look at it in the email, so I
> > am ok with that rule too if most people like it.
> 
> Starting with this (that code be reviewed, rather than than this
> author != committer that we seem to be fixated on) would make it much
> easier for me to pick up my side of the bargain, and do reviews for
> others.

As I said in the other mail:
If we find consensus in trying review and encouraging but not
requiring non-author-push, that'd be fine by me for a start.
If the reviewer is offering to push, we should let her.

> In turn, if we can find a process that is easier on the reviewers, then
> I have a chance of sourcing the 1300 reviews I would have needed last
> year.  For me that challenge is real, I've already spoken to one
> reviewer, who indicated he certainly could not handle it on his own.

You are certainly right in that this will be a challenge for
reviewers. Certainly there should not only be a single one for
your patches. Similarly with Metze who has had a share but not the
majority of his patches reviewed.

A trial period could tell whether we can cope with that
amount of productivity!... :-)

I think if we just say "Yeah, it would be nice to do code
reviews, let's see if we can change the coverage..." then
nothing much will happen. Hence the proposal for a committed
trial period.

Cheers - Michael

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 206 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20121015/2e3d98b0/attachment.pgp>