[PATCH v2] MAINTAINERS: Document maintainers for xen/common/

[PATCH v2] MAINTAINERS: Document maintainers for xen/common/

[Ian Jackson]

This is a copy of the `THE REST' entry but with my own entry removed
(as I do not normally review hypervisor patches) and Andrew Cooper's
added (which seems appropriate given his status as x86 hypervisor
maintainer).

The effect is that patches touching xen/common/ will no longer be CC'd
to me; but also that Ian C, Andrew, Jan, Keir and Tim will get _all_
patches touching xen/common/, not just ones which match one of their
existing more specific entries.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Jan Beulich <jbeulich@suse.com>
CC: Keir Fraser <keir@xen.org>
CC: Tim Deegan <tim@xen.org>
CC: Julien Grall <julien.grall@citrix.com>
CC: David Vrabel <david.vrabel@citrix.com>
CC: Andrew Cooper <andrew.cooper3@citrix.com>

---
v2: A completely different approach following discussion with Ian C.
---
 MAINTAINERS |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index a6bece6..e35c1e9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -404,6 +404,16 @@ F:  xen/include/xsm/
 F:  xen/xsm/
 F:  docs/misc/xsm-flask.txt
 
+HYPERVISOR CORE
+M:	Jan Beulich <jbeulich@suse.com>
+M:	Tim Deegan <tim@xen.org>
+M:	Keir Fraser <keir@xen.org>
+M:	Ian Campbell <ian.campbell@citrix.com>
+M:	Andrew Cooper <andrew.cooper3@citrix.com>
+L:	xen-devel@lists.xen.org
+S:	Supported
+F:	xen/common/
+
 THE REST
 M:	Ian Campbell <ian.campbell@citrix.com>
 M:	Ian Jackson <ian.jackson@eu.citrix.com>
-- 
1.7.10.4

Re: [PATCH v2] MAINTAINERS: Document maintainers for xen/common/

[Andrew Cooper]

On 17/09/15 12:53, Ian Jackson wrote:
> This is a copy of the `THE REST' entry but with my own entry removed
> (as I do not normally review hypervisor patches) and Andrew Cooper's
> added (which seems appropriate given his status as x86 hypervisor
> maintainer).
>
> The effect is that patches touching xen/common/ will no longer be CC'd
> to me; but also that Ian C, Andrew, Jan, Keir and Tim will get _all_
> patches touching xen/common/, not just ones which match one of their
> existing more specific entries.
>
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Ian Campbell <ian.campbell@citrix.com>
> CC: Jan Beulich <jbeulich@suse.com>
> CC: Keir Fraser <keir@xen.org>
> CC: Tim Deegan <tim@xen.org>
> CC: Julien Grall <julien.grall@citrix.com>
> CC: David Vrabel <david.vrabel@citrix.com>
> CC: Andrew Cooper <andrew.cooper3@citrix.com>
>
> ---
> v2: A completely different approach following discussion with Ian C.
> ---
>  MAINTAINERS |   10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a6bece6..e35c1e9 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -404,6 +404,16 @@ F:  xen/include/xsm/
>  F:  xen/xsm/
>  F:  docs/misc/xsm-flask.txt
>  
> +HYPERVISOR CORE
> +M:	Jan Beulich <jbeulich@suse.com>
> +M:	Tim Deegan <tim@xen.org>
> +M:	Keir Fraser <keir@xen.org>
> +M:	Ian Campbell <ian.campbell@citrix.com>
> +M:	Andrew Cooper <andrew.cooper3@citrix.com>
> +L:	xen-devel@lists.xen.org
> +S:	Supported
> +F:	xen/common/

I am happy with the change in principle, but it should cover xen/ rather
than xen/common/ to also include otherwise unqualified items in
xen/drivers/.

~Andrew

Re: [PATCH v2] MAINTAINERS: Document maintainers for xen/common/

[Julien Grall]

On 17/09/15 12:59, Andrew Cooper wrote:
> On 17/09/15 12:53, Ian Jackson wrote:
>> This is a copy of the `THE REST' entry but with my own entry removed
>> (as I do not normally review hypervisor patches) and Andrew Cooper's
>> added (which seems appropriate given his status as x86 hypervisor
>> maintainer).
>>
>> The effect is that patches touching xen/common/ will no longer be CC'd
>> to me; but also that Ian C, Andrew, Jan, Keir and Tim will get _all_
>> patches touching xen/common/, not just ones which match one of their
>> existing more specific entries.
>>
>> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
>> CC: Ian Campbell <ian.campbell@citrix.com>
>> CC: Jan Beulich <jbeulich@suse.com>
>> CC: Keir Fraser <keir@xen.org>
>> CC: Tim Deegan <tim@xen.org>
>> CC: Julien Grall <julien.grall@citrix.com>
>> CC: David Vrabel <david.vrabel@citrix.com>
>> CC: Andrew Cooper <andrew.cooper3@citrix.com>
>>
>> ---
>> v2: A completely different approach following discussion with Ian C.
>> ---
>>  MAINTAINERS |   10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index a6bece6..e35c1e9 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -404,6 +404,16 @@ F:  xen/include/xsm/
>>  F:  xen/xsm/
>>  F:  docs/misc/xsm-flask.txt
>>  
>> +HYPERVISOR CORE
>> +M:	Jan Beulich <jbeulich@suse.com>
>> +M:	Tim Deegan <tim@xen.org>
>> +M:	Keir Fraser <keir@xen.org>
>> +M:	Ian Campbell <ian.campbell@citrix.com>
>> +M:	Andrew Cooper <andrew.cooper3@citrix.com>
>> +L:	xen-devel@lists.xen.org
>> +S:	Supported
>> +F:	xen/common/
> 
> I am happy with the change in principle, but it should cover xen/ rather
> than xen/common/ to also include otherwise unqualified items in
> xen/drivers/.

The first version of this patch was covering xen/. But the downside was
to CC all the "REST OF THE HYPERVISOR" group to any changes under xen/.
I.e you would be CCed on ARM changes for instance.

See http://lists.xen.org/archives/html/xen-devel/2015-09/msg02171.html

I know some maintainers which will complaining very quickly on the ML
and ask the contributors to use correctly scripts/get_maintainers.pl.

But the scrips doesn't do correctly his job for Xen...

Regards,

-- 
Julien Grall

Re: [PATCH v2] MAINTAINERS: Document maintainers for xen/common/

[Tim Deegan]

At 12:53 +0100 on 17 Sep (1442494439), Ian Jackson wrote:
> This is a copy of the `THE REST' entry but with my own entry removed
> (as I do not normally review hypervisor patches) and Andrew Cooper's
> added (which seems appropriate given his status as x86 hypervisor
> maintainer).
> 
> The effect is that patches touching xen/common/ will no longer be CC'd
> to me; but also that Ian C, Andrew, Jan, Keir and Tim will get _all_
> patches touching xen/common/, not just ones which match one of their
> existing more specific entries.

This has the same problem as the previous patch, on a smaller
scale.  There's no need for us all to be CC'd on code that already has
a maintainer (e.g. scheduler, xentrace, device-tree, tmem...)

I would rather your v1 plus an appropriate change to get_maintainers.

Cheers,

Tim.

> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Ian Campbell <ian.campbell@citrix.com>
> CC: Jan Beulich <jbeulich@suse.com>
> CC: Keir Fraser <keir@xen.org>
> CC: Tim Deegan <tim@xen.org>
> CC: Julien Grall <julien.grall@citrix.com>
> CC: David Vrabel <david.vrabel@citrix.com>
> CC: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> ---
> v2: A completely different approach following discussion with Ian C.
> ---
>  MAINTAINERS |   10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a6bece6..e35c1e9 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -404,6 +404,16 @@ F:  xen/include/xsm/
>  F:  xen/xsm/
>  F:  docs/misc/xsm-flask.txt
>  
> +HYPERVISOR CORE
> +M:	Jan Beulich <jbeulich@suse.com>
> +M:	Tim Deegan <tim@xen.org>
> +M:	Keir Fraser <keir@xen.org>
> +M:	Ian Campbell <ian.campbell@citrix.com>
> +M:	Andrew Cooper <andrew.cooper3@citrix.com>
> +L:	xen-devel@lists.xen.org
> +S:	Supported
> +F:	xen/common/
> +
>  THE REST
>  M:	Ian Campbell <ian.campbell@citrix.com>
>  M:	Ian Jackson <ian.jackson@eu.citrix.com>
> -- 
> 1.7.10.4
> 

Re: [PATCH v2] MAINTAINERS: Document maintainers for xen/common/

[Ian Jackson]

Tim Deegan writes ("Re: [PATCH v2] MAINTAINERS: Document maintainers for xen/common/"):
> I would rather your v1 plus an appropriate change to get_maintainers.

I am happy to implement in get_maintainers whatever is decided.

At the moment there is one kind of fallback:

(i) `THE REST' gets CCd iff no file pointed to any other maintainer

What you may be proposing is a subtly different kind of fallback:

(ii) `REST OF THE HYPERVISOR' gets CCd iff the patch touches any files
   in xen/ which do not have another maintainer

The difference is apparent in patches which touch both more specific
and less specific items.

Alternatively maybe you are proposing that we should have two
hierarchical fallbacks: `REST OF THE HYPERVISOR' gets CCd if any file
is in xen/ is touched but is skipped BUT only if no file pointed to
another maintainer.

Ian.

Re: [PATCH v2] MAINTAINERS: Document maintainers for xen/common/

[Jan Beulich]

>>> Ian Jackson <Ian.Jackson@eu.citrix.com> 09/17/15 8:02 PM >>>
>Tim Deegan writes ("Re: [PATCH v2] MAINTAINERS: Document maintainers for xen/common/"):
>> I would rather your v1 plus an appropriate change to get_maintainers.
>
>I am happy to implement in get_maintainers whatever is decided.
>
>At the moment there is one kind of fallback:
>
>(i) `THE REST' gets CCd iff no file pointed to any other maintainer
>
>What you may be proposing is a subtly different kind of fallback:
>
>(ii) `REST OF THE HYPERVISOR' gets CCd iff the patch touches any files
   >in xen/ which do not have another maintainer
>
>The difference is apparent in patches which touch both more specific
>and less specific items.
>
>Alternatively maybe you are proposing that we should have two
>hierarchical fallbacks: `REST OF THE HYPERVISOR' gets CCd if any file
>is in xen/ is touched but is skipped BUT only if no file pointed to
>another maintainer.

I have to admit being confused by the wording, namely the two "but", but also the
last part of the sentence (repeating text from (i) above). For the latter - isn't the
current model "CC if any file pointed to be no other maintainer"? I'd basically see
_that_ extended in the hierarchical(?) manner you mention. Since - afaics - the
result is the same whether it's done hierarchically or as alternatives, I'm not sure
"hierarchical" actually applies.

Jan

Re: [PATCH v2] MAINTAINERS: Document maintainers for xen/common/

[Tim Deegan]

At 19:02 +0100 on 17 Sep (1442516564), Ian Jackson wrote:
> Tim Deegan writes ("Re: [PATCH v2] MAINTAINERS: Document maintainers for xen/common/"):
> > I would rather your v1 plus an appropriate change to get_maintainers.
> 
> I am happy to implement in get_maintainers whatever is decided.
> 
> At the moment there is one kind of fallback:
> 
> (i) `THE REST' gets CCd iff no file pointed to any other maintainer
> 
> What you may be proposing is a subtly different kind of fallback:
> 
> (ii) `REST OF THE HYPERVISOR' gets CCd iff the patch touches any files
>    in xen/ which do not have another maintainer

Right, I see.  Yes, that is what I was proposing, though thinking
about it I'm not so sure any more.  There seem to be three
possibilities -- filter everything per-file:

 for each file touched:
     M := 'normal' maintainers
     if M is empty and the file is in xen/, M := REST OF HYPERVISOR
     if M is empty, M := THE REST
     CCLIST += M

Filter xen/ per-file but leave THE REST as it is now:

 for each file touched:
     M := 'normal' maintainers
     if M is empty and the file is in xen/, M := REST OF HYPERVISOR
     CCLIST += M
 if CCLIST is empty, CCLIST := THE REST

or only use the fallbacks if nothing in the patch has a real
maintainer: 

 for each file touched:
     CCLIST += 'normal' maintainers
 if CCLIST is empty and any file is in xen/, CCLIST := REST OF HYPERVISOR
 if CCLIST is empty, CCLIST := THE REST

There's a tradeoff between how noisy it gets for the fallback
maintainers and wanting to make sure that someone relevant is always
CC'd.

I think on balance we can rely on the maintainers to ask for second
opinions if they find a patch that touches a lot of common code,
i.e. I prefer the third option.

Cheers,

Tim.