Re: [Xen-devel] [PATCH] MAINTAINERS: Add explicit check-in policy section

[Jan Beulich <jbeulich@suse.com>]

On 07.01.2020 13:03, George Dunlap wrote:
> DISCUSSION
> 
> This seems to be a change from people's understanding of the current
> policy.  Most people's understanding of the current policy seems to be:
> 
> 1.  In order to get a change to a given file committed, it must have
> an Ack or Review from at least one *maintainer* of that file other
> than the submitter.
> 
> 2. In the case where a file has only one maintainer, it must have an
> Ack or Review from a "nested" maintainer.
> 
> I.e., if I submitted something to x86/mm, it would require an Ack from
> Jan or Andy, or (in exceptional circumstances) The Rest; but an Ack from
> (say) Roger or Juergen wouldn't suffice.
> 
> Let's call this the "maintainer-ack" approach (because it must have an
> ack or r-b from a maintainer to be checked in), and the proposal in
> this patch the "maintainer-approval" (since SoB from a maintainer
> indicates approval).
> 
> The core issue I have with "maintainer-ack" is that it makes the
> maintainer less privileged with regard to writing code than
> non-maintainers.  If component X has maintainers A and B, then a
> non-maintainer can have code checked in if reviewed either by A or B.
> If A or B wants code checked in, they have to wait for exactly one
> person to review it.
> 
> In fact, if B is quite busy, the easiest way for A really to get their
> code checked in might be to hand it to a non-maintainer N, and ask N
> to submit it as their own.  Then A can Ack the patches and check them
> in.
> 
> The current system, therefore, either sets up a perverse incentive (if
> you think the behavior described above is unacceptable) or unnecessary
> bureaucracy (if you think it's acceptable).  Either way I think we
> should set up our system to avoid it.

I much appreciate this initiative of yours.

> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -104,7 +104,53 @@ Descriptions of section entries:
>  	   xen-maintainers-<version format number of this file>
>  
>  
> -The meaning of nesting:
> +	Check-in policy
> +	===============
> +
> +In order for a patch to be checked in, in general, several conditions
> +must be met:
> +
> +1. In order to get a change to a given file committed, it must have
> +   the approval of at least one maintainer of that file.
> +
> +   A patch of course needs Acks from the maintainers of each file that
> +   it changes; so a patch which changes xen/arch/x86/traps.c,
> +   xen/arch/x86/mm/p2m.c, and xen/arch/x86/mm/shadow/multi.c would
> +   require an Ack from each of the three sets of maintainers.
> +
> +   See below for rules on nested maintainership.
> +
> +2. It must have an Acked-by or a Reviewed-by from someone other than
> +   the submitter.

I'd like to propose some further distinction here, albeit I'm not sure
this isn't implied anyway. It might be that making explicit the
distinction between A-b and R-b is sufficient - our current common
understanding looks to be that only maintainers can "ack", and others
would "review". Since the latter is implying a more thorough look at a
patch, I think it wouldn't be right to allow (quoting text further
down) "anyone in the community" to ack a random patch (I could probably
talk my son into ack-ing my patches ;-) ). Perhaps, rather than
limiting acks to maintainers of the changed code, we could extend this
to maintainers of just some code for maintainer submitted patches (i.e.
anyone named as M: at least once in ./MAINTAINERS)? People outside of
whatever subset we might pick would be eligible to offer R-b only,
implying of course that they actually did do a review.

> +3. Sufficient time and/or warning must have been given for anyone to
> +   respond.  This depends in large part upon the urgency and nature of
> +   the patch.  For a straightforward uncontroversial patch, a day or
> +   two is sufficient; for a controversial patch, perhaps waiting a
> +   week and then saying "I intend to check this in tomorrow unless I
> +   hear otherwise".

To me as non-native speaker, this last sentence looks incomplete (as
in missing e.g. "would be appropriate" at the end), or alternatively
it would feel like wanting the two "ing" dropped from the verbs.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel