Improving how we work with external contributors: Make double-reviews optional in review trial

[Jelmer Vernooij]

On Tue, Jul 30, 2013 at 01:36:16PM +1200, Andrew Bartlett wrote:
> This came up for me a couple of months ago, where it was pointed out to
> me that our review trial, which I've been a part of (at least as regards
> my own patches) requires that two team members to review any patch,
> which isn't itself authored by a team member.
> 
> I think this creates a unnecessary barrier between team members and
> non-team members, has not been universally applied even in the voluntary
> trial, and I would like to propose that we change 
> https://wiki.samba.org/index.php/CodeReview to say:
> 
> proposed future policy 
> The ultimate policy we (or some of us) would like to establish for
> mandatory code code review is this:
> 
>       * Each commit in master should have been reviewed by a samba team
>         member. For this purpose, involvement in patch creation is
>         considered as reviewing.
> 
> I wrote at length about my concerns here before, and aside from Micheal
> Adam, I didn't get any response.  In short, the reason I propose this is
> to encourage those who are not members of the team, by avoiding barriers
> to their participation, and to simplify the process for team members who
> push these changes, by not requiring the co-ordination of both two team
> members and the outside contributor.
> 
> My previous comments are here:
> https://lists.samba.org/archive/samba-technical/2013-June/092980.html
> 
> Certainly I've almost ceased actively applying patches from external
> contributors - with other calls on my time, the additional burden here
> means I've left to others what I was previously actively merging. 
+1

> None of our review policy excuses us as team members from asking for
> further review of changes that we are not sure of, but the same applies
> to patches by team members.  
*THIS*

It makes sense to keep the process for reviews lightweight, and expect
team members to do the right thing (e.g. ask for more review for
potentially controversial changes). If that turns out to be
problematic then we can always discuss it again.

Cheers,

Jelmer

(whose absence is not related to the review process...)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20130731/5e87fe8b/attachment.pgp>