[PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Lee Jones]

Since eafbaac ("MAINTAINERS: Add "R:" designated-reviewers tag") we
have been able to tag specific people as Reviewers.  These are key
individuals who are tasked with or volunteer to review code submitted
to a subsystem or specific file.  However, according to MAINTAINERS
we have 1046 Maintainers and only a mere 22 Reviewers.  I believe
these numbers to be incorrect, as many of these Maintainers are in
fact Reviewers.

I have taken the time to identify some of the Reviewers who pertain
to subsystems which I look after, and have changed their status from
Maintainer (collector of patches) to Reviewer (reviewer of code).

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---

 MAINTAINERS | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 9de185d..07bc92f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -392,7 +392,7 @@ F:	drivers/media/i2c/adp1653.c
 F:	include/media/adp1653.h
 
 ADP5520 BACKLIGHT DRIVER WITH IO EXPANDER (ADP5520/ADP5501)
-M:	Michael Hennerich <michael.hennerich@analog.com>
+R:	Michael Hennerich <michael.hennerich@analog.com>
 W:	http://wiki.analog.com/ADP5520
 W:	http://ez.analog.com/community/linux-device-drivers
 S:	Supported
@@ -411,7 +411,7 @@ F:	drivers/input/keyboard/adp5588-keys.c
 F:	drivers/gpio/gpio-adp5588.c
 
 ADP8860 BACKLIGHT DRIVER (ADP8860/ADP8861/ADP8863)
-M:	Michael Hennerich <michael.hennerich@analog.com>
+R:	Michael Hennerich <michael.hennerich@analog.com>
 W:	http://wiki.analog.com/ADP8860
 W:	http://ez.analog.com/community/linux-device-drivers
 S:	Supported
@@ -2049,8 +2049,8 @@ S:	Maintained
 F:	drivers/net/wireless/b43legacy/
 
 BACKLIGHT CLASS/SUBSYSTEM
-M:	Jingoo Han <jingoohan1@gmail.com>
 M:	Lee Jones <lee.jones@linaro.org>
+R:	Jingoo Han <jingoohan1@gmail.com>
 S:	Maintained
 F:	drivers/video/backlight/
 F:	include/linux/backlight.h
@@ -3364,7 +3364,7 @@ F:	include/linux/dm-*.h
 F:	include/uapi/linux/dm-*.h
 
 DIALOG SEMICONDUCTOR DRIVERS
-M:	Support Opensource <support.opensource@diasemi.com>
+R:	Support Opensource <support.opensource@diasemi.com>
 W:	http://www.dialog-semiconductor.com/products
 S:	Supported
 F:	Documentation/hwmon/da90??
@@ -5212,7 +5212,7 @@ S:	Orphan
 F:	drivers/scsi/ips.*
 
 ICH LPC AND GPIO DRIVER
-M:	Peter Tyser <ptyser@xes-inc.com>
+R:	Peter Tyser <ptyser@xes-inc.com>
 S:	Maintained
 F:	drivers/mfd/lpc_ich.c
 F:	drivers/gpio/gpio-ich.c
@@ -6855,7 +6855,7 @@ F:	include/linux/mcb.h
 F:	Documentation/men-chameleon-bus.txt
 
 MEN F21BMC (Board Management Controller)
-M:	Andreas Werner <andreas.werner@men.de>
+R:	Andreas Werner <andreas.werner@men.de>
 S:	Supported
 F:	drivers/mfd/menf21bmc.c
 F:	drivers/watchdog/menf21bmc_wdt.c
@@ -9009,8 +9009,8 @@ S:	Maintained
 F:	drivers/video/fbdev/s3c-fb.c
 
 SAMSUNG MULTIFUNCTION PMIC DEVICE DRIVERS
-M:	Sangbeom Kim <sbkim73@samsung.com>
-M:	Krzysztof Kozlowski <k.kozlowski@samsung.com>
+R:	Sangbeom Kim <sbkim73@samsung.com>
+R:	Krzysztof Kozlowski <k.kozlowski@samsung.com>
 L:	linux-kernel@vger.kernel.org
 L:	linux-samsung-soc@vger.kernel.org
 S:	Supported
@@ -10434,20 +10434,20 @@ F:	sound/soc/codecs/lm49453*
 F:	sound/soc/codecs/isabelle*
 
 TI LP855x BACKLIGHT DRIVER
-M:	Milo Kim <milo.kim@ti.com>
+R:	Milo Kim <milo.kim@ti.com>
 S:	Maintained
 F:	Documentation/backlight/lp855x-driver.txt
 F:	drivers/video/backlight/lp855x_bl.c
 F:	include/linux/platform_data/lp855x.h
 
 TI LP8727 CHARGER DRIVER
-M:	Milo Kim <milo.kim@ti.com>
+R:	Milo Kim <milo.kim@ti.com>
 S:	Maintained
 F:	drivers/power/lp8727_charger.c
 F:	include/linux/platform_data/lp8727.h
 
 TI LP8788 MFD DRIVER
-M:	Milo Kim <milo.kim@ti.com>
+R:	Milo Kim <milo.kim@ti.com>
 S:	Maintained
 F:	drivers/iio/adc/lp8788_adc.c
 F:	drivers/leds/leds-lp8788.c
-- 
1.9.1

[PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Lee Jones]

Since eafbaac ("MAINTAINERS: Add "R:" designated-reviewers tag") we
have been able to tag specific people as Reviewers.  These are key
individuals who are tasked with or volunteer to review code submitted
to a subsystem or specific file.  However, according to MAINTAINERS
we have 1046 Maintainers and only a mere 22 Reviewers.  I believe
these numbers to be incorrect, as many of these Maintainers are in
fact Reviewers.

I have taken the time to identify some of the Reviewers who pertain
to subsystems which I look after, and have changed their status from
Maintainer (collector of patches) to Reviewer (reviewer of code).

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---

 MAINTAINERS | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 9de185d..07bc92f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -392,7 +392,7 @@ F:	drivers/media/i2c/adp1653.c
 F:	include/media/adp1653.h
 
 ADP5520 BACKLIGHT DRIVER WITH IO EXPANDER (ADP5520/ADP5501)
-M:	Michael Hennerich <michael.hennerich@analog.com>
+R:	Michael Hennerich <michael.hennerich@analog.com>
 W:	http://wiki.analog.com/ADP5520
 W:	http://ez.analog.com/community/linux-device-drivers
 S:	Supported
@@ -411,7 +411,7 @@ F:	drivers/input/keyboard/adp5588-keys.c
 F:	drivers/gpio/gpio-adp5588.c
 
 ADP8860 BACKLIGHT DRIVER (ADP8860/ADP8861/ADP8863)
-M:	Michael Hennerich <michael.hennerich@analog.com>
+R:	Michael Hennerich <michael.hennerich@analog.com>
 W:	http://wiki.analog.com/ADP8860
 W:	http://ez.analog.com/community/linux-device-drivers
 S:	Supported
@@ -2049,8 +2049,8 @@ S:	Maintained
 F:	drivers/net/wireless/b43legacy/
 
 BACKLIGHT CLASS/SUBSYSTEM
-M:	Jingoo Han <jingoohan1@gmail.com>
 M:	Lee Jones <lee.jones@linaro.org>
+R:	Jingoo Han <jingoohan1@gmail.com>
 S:	Maintained
 F:	drivers/video/backlight/
 F:	include/linux/backlight.h
@@ -3364,7 +3364,7 @@ F:	include/linux/dm-*.h
 F:	include/uapi/linux/dm-*.h
 
 DIALOG SEMICONDUCTOR DRIVERS
-M:	Support Opensource <support.opensource@diasemi.com>
+R:	Support Opensource <support.opensource@diasemi.com>
 W:	http://www.dialog-semiconductor.com/products
 S:	Supported
 F:	Documentation/hwmon/da90??
@@ -5212,7 +5212,7 @@ S:	Orphan
 F:	drivers/scsi/ips.*
 
 ICH LPC AND GPIO DRIVER
-M:	Peter Tyser <ptyser@xes-inc.com>
+R:	Peter Tyser <ptyser@xes-inc.com>
 S:	Maintained
 F:	drivers/mfd/lpc_ich.c
 F:	drivers/gpio/gpio-ich.c
@@ -6855,7 +6855,7 @@ F:	include/linux/mcb.h
 F:	Documentation/men-chameleon-bus.txt
 
 MEN F21BMC (Board Management Controller)
-M:	Andreas Werner <andreas.werner@men.de>
+R:	Andreas Werner <andreas.werner@men.de>
 S:	Supported
 F:	drivers/mfd/menf21bmc.c
 F:	drivers/watchdog/menf21bmc_wdt.c
@@ -9009,8 +9009,8 @@ S:	Maintained
 F:	drivers/video/fbdev/s3c-fb.c
 
 SAMSUNG MULTIFUNCTION PMIC DEVICE DRIVERS
-M:	Sangbeom Kim <sbkim73@samsung.com>
-M:	Krzysztof Kozlowski <k.kozlowski@samsung.com>
+R:	Sangbeom Kim <sbkim73@samsung.com>
+R:	Krzysztof Kozlowski <k.kozlowski@samsung.com>
 L:	linux-kernel at vger.kernel.org
 L:	linux-samsung-soc at vger.kernel.org
 S:	Supported
@@ -10434,20 +10434,20 @@ F:	sound/soc/codecs/lm49453*
 F:	sound/soc/codecs/isabelle*
 
 TI LP855x BACKLIGHT DRIVER
-M:	Milo Kim <milo.kim@ti.com>
+R:	Milo Kim <milo.kim@ti.com>
 S:	Maintained
 F:	Documentation/backlight/lp855x-driver.txt
 F:	drivers/video/backlight/lp855x_bl.c
 F:	include/linux/platform_data/lp855x.h
 
 TI LP8727 CHARGER DRIVER
-M:	Milo Kim <milo.kim@ti.com>
+R:	Milo Kim <milo.kim@ti.com>
 S:	Maintained
 F:	drivers/power/lp8727_charger.c
 F:	include/linux/platform_data/lp8727.h
 
 TI LP8788 MFD DRIVER
-M:	Milo Kim <milo.kim@ti.com>
+R:	Milo Kim <milo.kim@ti.com>
 S:	Maintained
 F:	drivers/iio/adc/lp8788_adc.c
 F:	drivers/leds/leds-lp8788.c
-- 
1.9.1

Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 896 bytes --]

Hi,

On Tue, Oct 27, 2015 at 03:42:37PM +0000, Lee Jones wrote:
> Since eafbaac ("MAINTAINERS: Add "R:" designated-reviewers tag") we
> have been able to tag specific people as Reviewers.  These are key
> individuals who are tasked with or volunteer to review code submitted
> to a subsystem or specific file.  However, according to MAINTAINERS
> we have 1046 Maintainers and only a mere 22 Reviewers.  I believe
> these numbers to be incorrect, as many of these Maintainers are in
> fact Reviewers.
> 
> I have taken the time to identify some of the Reviewers who pertain
> to subsystems which I look after, and have changed their status from
> Maintainer (collector of patches) to Reviewer (reviewer of code).

[for drivers/power/*]
Acked-By: Sebastian Reichel <sre@kernel.org>

I think you should CC the people, which are changed from "M:" to
"R:", though.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Sebastian Reichel]

Hi,

On Tue, Oct 27, 2015 at 03:42:37PM +0000, Lee Jones wrote:
> Since eafbaac ("MAINTAINERS: Add "R:" designated-reviewers tag") we
> have been able to tag specific people as Reviewers.  These are key
> individuals who are tasked with or volunteer to review code submitted
> to a subsystem or specific file.  However, according to MAINTAINERS
> we have 1046 Maintainers and only a mere 22 Reviewers.  I believe
> these numbers to be incorrect, as many of these Maintainers are in
> fact Reviewers.
> 
> I have taken the time to identify some of the Reviewers who pertain
> to subsystems which I look after, and have changed their status from
> Maintainer (collector of patches) to Reviewer (reviewer of code).

[for drivers/power/*]
Acked-By: Sebastian Reichel <sre@kernel.org>

I think you should CC the people, which are changed from "M:" to
"R:", though.

-- Sebastian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20151027/ddea2298/attachment-0001.sig>

Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Lee Jones]

On Tue, 27 Oct 2015, Sebastian Reichel wrote:
> On Tue, Oct 27, 2015 at 03:42:37PM +0000, Lee Jones wrote:
> > Since eafbaac ("MAINTAINERS: Add "R:" designated-reviewers tag") we
> > have been able to tag specific people as Reviewers.  These are key
> > individuals who are tasked with or volunteer to review code submitted
> > to a subsystem or specific file.  However, according to MAINTAINERS
> > we have 1046 Maintainers and only a mere 22 Reviewers.  I believe
> > these numbers to be incorrect, as many of these Maintainers are in
> > fact Reviewers.
> > 
> > I have taken the time to identify some of the Reviewers who pertain
> > to subsystems which I look after, and have changed their status from
> > Maintainer (collector of patches) to Reviewer (reviewer of code).
> 
> [for drivers/power/*]
> Acked-By: Sebastian Reichel <sre@kernel.org>

Thanks.

> I think you should CC the people, which are changed from "M:" to
> "R:", though.

Yes, makes sense.

I'd like to collect some Maintainer Acks first though.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Lee Jones]

On Tue, 27 Oct 2015, Sebastian Reichel wrote:
> On Tue, Oct 27, 2015 at 03:42:37PM +0000, Lee Jones wrote:
> > Since eafbaac ("MAINTAINERS: Add "R:" designated-reviewers tag") we
> > have been able to tag specific people as Reviewers.  These are key
> > individuals who are tasked with or volunteer to review code submitted
> > to a subsystem or specific file.  However, according to MAINTAINERS
> > we have 1046 Maintainers and only a mere 22 Reviewers.  I believe
> > these numbers to be incorrect, as many of these Maintainers are in
> > fact Reviewers.
> > 
> > I have taken the time to identify some of the Reviewers who pertain
> > to subsystems which I look after, and have changed their status from
> > Maintainer (collector of patches) to Reviewer (reviewer of code).
> 
> [for drivers/power/*]
> Acked-By: Sebastian Reichel <sre@kernel.org>

Thanks.

> I think you should CC the people, which are changed from "M:" to
> "R:", though.

Yes, makes sense.

I'd like to collect some Maintainer Acks first though.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Joe Perches]

On Tue, 2015-10-27 at 18:15 +0000, Lee Jones wrote:
> On Tue, 27 Oct 2015, Sebastian Reichel wrote:
> > On Tue, Oct 27, 2015 at 03:42:37PM +0000, Lee Jones wrote:
> > > Since eafbaac ("MAINTAINERS: Add "R:" designated-reviewers tag") we
> > > have been able to tag specific people as Reviewers.  These are key
> > > individuals who are tasked with or volunteer to review code submitted
> > > to a subsystem or specific file.  However, according to MAINTAINERS
> > > we have 1046 Maintainers and only a mere 22 Reviewers.  I believe
> > > these numbers to be incorrect, as many of these Maintainers are in
> > > fact Reviewers.

Most entries in MAINTAINERS seem to be vanity entries than actual
active participants.  A person typically writes a driver, adds a
MAINTAINER entry, then forgets about it and/or the hardware becomes
outdated.

> > > I have taken the time to identify some of the Reviewers who pertain
> > > to subsystems which I look after, and have changed their status from
> > > Maintainer (collector of patches) to Reviewer (reviewer of code).
> > 
> > [for drivers/power/*]
> > Acked-By: Sebastian Reichel <sre@kernel.org>
> 
> Thanks.
> 
> > I think you should CC the people, which are changed from "M:" to
> > "R:", though.
> 
> Yes, makes sense.
> 
> I'd like to collect some Maintainer Acks first though.

I think people from organizations like Samsung are actual
maintainers not reviewers.

Their drivers are not thrown over a wall and forgotten.

[PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Joe Perches]

On Tue, 2015-10-27 at 18:15 +0000, Lee Jones wrote:
> On Tue, 27 Oct 2015, Sebastian Reichel wrote:
> > On Tue, Oct 27, 2015 at 03:42:37PM +0000, Lee Jones wrote:
> > > Since eafbaac ("MAINTAINERS: Add "R:" designated-reviewers tag") we
> > > have been able to tag specific people as Reviewers.  These are key
> > > individuals who are tasked with or volunteer to review code submitted
> > > to a subsystem or specific file.  However, according to MAINTAINERS
> > > we have 1046 Maintainers and only a mere 22 Reviewers.  I believe
> > > these numbers to be incorrect, as many of these Maintainers are in
> > > fact Reviewers.

Most entries in MAINTAINERS seem to be vanity entries than actual
active participants.  A person typically writes a driver, adds a
MAINTAINER entry, then forgets about it and/or the hardware becomes
outdated.

> > > I have taken the time to identify some of the Reviewers who pertain
> > > to subsystems which I look after, and have changed their status from
> > > Maintainer (collector of patches) to Reviewer (reviewer of code).
> > 
> > [for drivers/power/*]
> > Acked-By: Sebastian Reichel <sre@kernel.org>
> 
> Thanks.
> 
> > I think you should CC the people, which are changed from "M:" to
> > "R:", though.
> 
> Yes, makes sense.
> 
> I'd like to collect some Maintainer Acks first though.

I think people from organizations like Samsung are actual
maintainers not reviewers.

Their drivers are not thrown over a wall and forgotten.

Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Krzysztof Kozlowski]

2015-10-28 3:44 GMT+09:00 Joe Perches <joe@perches.com>:
>
> On Tue, 2015-10-27 at 18:15 +0000, Lee Jones wrote:
> > On Tue, 27 Oct 2015, Sebastian Reichel wrote:> >
> > > I think you should CC the people, which are changed from "M:" to
> > > "R:", though.
> >
> > Yes, makes sense.
> >
> > I'd like to collect some Maintainer Acks first though.
>
> I think people from organizations like Samsung are actual
> maintainers not reviewers.
>
> Their drivers are not thrown over a wall and forgotten.

At least for Samsung Multifunction PMIC drivers (and some of Maxim
MUICs and PMICs) these are actively used by us in existing and new
products. They are also continuously extended and actually maintained.
This means that it is not only about review of new patches but also
about caring that nothing will become broken.

I would prefer to leave the "SAMSUNG MULTIFUNCTION PMIC DEVICE
DRIVERS" entry as is - maintainers.

Best regards,
Krzysztof

[PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Krzysztof Kozlowski]

2015-10-28 3:44 GMT+09:00 Joe Perches <joe@perches.com>:
>
> On Tue, 2015-10-27 at 18:15 +0000, Lee Jones wrote:
> > On Tue, 27 Oct 2015, Sebastian Reichel wrote:> >
> > > I think you should CC the people, which are changed from "M:" to
> > > "R:", though.
> >
> > Yes, makes sense.
> >
> > I'd like to collect some Maintainer Acks first though.
>
> I think people from organizations like Samsung are actual
> maintainers not reviewers.
>
> Their drivers are not thrown over a wall and forgotten.

At least for Samsung Multifunction PMIC drivers (and some of Maxim
MUICs and PMICs) these are actively used by us in existing and new
products. They are also continuously extended and actually maintained.
This means that it is not only about review of new patches but also
about caring that nothing will become broken.

I would prefer to leave the "SAMSUNG MULTIFUNCTION PMIC DEVICE
DRIVERS" entry as is - maintainers.

Best regards,
Krzysztof

Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Lee Jones]

On Tue, 2015-10-27 at 18:15 +0000, Lee Jones wrote:
> On Tue, 27 Oct 2015, Sebastian Reichel wrote:
> > On Tue, Oct 27, 2015 at 03:42:37PM +0000, Lee Jones wrote:
> > > Since eafbaac ("MAINTAINERS: Add "R:" designated-reviewers tag") we
> > > have been able to tag specific people as Reviewers.  These are key
> > > individuals who are tasked with or volunteer to review code submitted
> > > to a subsystem or specific file.  However, according to MAINTAINERS
> > > we have 1046 Maintainers and only a mere 22 Reviewers.  I believe
> > > these numbers to be incorrect, as many of these Maintainers are in
> > > fact Reviewers.

Most entries in MAINTAINERS seem to be vanity entries than actual
active participants.  A person typically writes a driver, adds a
MAINTAINER entry, then forgets about it and/or the hardware becomes
outdated.

This I agree with.

On Wed, 28 Oct 2015, Krzysztof Kozlowski wrote:
> 2015-10-28 3:44 GMT+09:00 Joe Perches <joe@perches.com>:
> > On Tue, 2015-10-27 at 18:15 +0000, Lee Jones wrote:
> > > On Tue, 27 Oct 2015, Sebastian Reichel wrote:> >
> > > > I think you should CC the people, which are changed from "M:" to
> > > > "R:", though.
> > >
> > > Yes, makes sense.
> > >
> > > I'd like to collect some Maintainer Acks first though.
> >
> > I think people from organizations like Samsung are actual
> > maintainers not reviewers.

So this all hinges on how we are describing Maintainers and Reviewers.

My personal definition (until convinced otherwise) is that Reviewers
care about their particular subsystem and/or files.  They conduct code
reviews to ensure nothing gets broken and the code base stays in best
possible state of worthiness.  On the other hand Maintainers usually
conduct themselves as Reviewers but also have 'maintainership' duties
as well; such as applying patches, *maintaining*, testing, rebasing,
etc, an upstream branch and ultimately sending pull-requests to higher
level Maintainers i.e. Linus.  Maintainers also have the ultimate say
(unless over-ruled by Linus etc) over what gets applied.

> > Their drivers are not thrown over a wall and forgotten.
> 
> At least for Samsung Multifunction PMIC drivers (and some of Maxim
> MUICs and PMICs) these are actively used by us in existing and new
> products. They are also continuously extended and actually maintained.
> This means that it is not only about review of new patches but also
> about caring that nothing will become broken.

Exactly.  This what I expect of any good code Reviewer.

> I would prefer to leave the "SAMSUNG MULTIFUNCTION PMIC DEVICE
> DRIVERS" entry as is - maintainers.

But you aren't maintaining the driver i.e. you don't collect patches
and *maintain* them on an upstream branch.  Granted some of you guys
are doing a great job of maintaining branches on your downstream or
BSP kernels, but conduct a Reviewer type role for upstream.

You guys are pushing back like this is some kind of demotion.  That's
not the case at all.  All it does is better describe the (very worthy)
function you *actually* provide.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Lee Jones]

On Tue, 2015-10-27 at 18:15 +0000, Lee Jones wrote:
> On Tue, 27 Oct 2015, Sebastian Reichel wrote:
> > On Tue, Oct 27, 2015 at 03:42:37PM +0000, Lee Jones wrote:
> > > Since eafbaac ("MAINTAINERS: Add "R:" designated-reviewers tag") we
> > > have been able to tag specific people as Reviewers.  These are key
> > > individuals who are tasked with or volunteer to review code submitted
> > > to a subsystem or specific file.  However, according to MAINTAINERS
> > > we have 1046 Maintainers and only a mere 22 Reviewers.  I believe
> > > these numbers to be incorrect, as many of these Maintainers are in
> > > fact Reviewers.

Most entries in MAINTAINERS seem to be vanity entries than actual
active participants.  A person typically writes a driver, adds a
MAINTAINER entry, then forgets about it and/or the hardware becomes
outdated.

This I agree with.

On Wed, 28 Oct 2015, Krzysztof Kozlowski wrote:
> 2015-10-28 3:44 GMT+09:00 Joe Perches <joe@perches.com>:
> > On Tue, 2015-10-27 at 18:15 +0000, Lee Jones wrote:
> > > On Tue, 27 Oct 2015, Sebastian Reichel wrote:> >
> > > > I think you should CC the people, which are changed from "M:" to
> > > > "R:", though.
> > >
> > > Yes, makes sense.
> > >
> > > I'd like to collect some Maintainer Acks first though.
> >
> > I think people from organizations like Samsung are actual
> > maintainers not reviewers.

So this all hinges on how we are describing Maintainers and Reviewers.

My personal definition (until convinced otherwise) is that Reviewers
care about their particular subsystem and/or files.  They conduct code
reviews to ensure nothing gets broken and the code base stays in best
possible state of worthiness.  On the other hand Maintainers usually
conduct themselves as Reviewers but also have 'maintainership' duties
as well; such as applying patches, *maintaining*, testing, rebasing,
etc, an upstream branch and ultimately sending pull-requests to higher
level Maintainers i.e. Linus.  Maintainers also have the ultimate say
(unless over-ruled by Linus etc) over what gets applied.

> > Their drivers are not thrown over a wall and forgotten.
> 
> At least for Samsung Multifunction PMIC drivers (and some of Maxim
> MUICs and PMICs) these are actively used by us in existing and new
> products. They are also continuously extended and actually maintained.
> This means that it is not only about review of new patches but also
> about caring that nothing will become broken.

Exactly.  This what I expect of any good code Reviewer.

> I would prefer to leave the "SAMSUNG MULTIFUNCTION PMIC DEVICE
> DRIVERS" entry as is - maintainers.

But you aren't maintaining the driver i.e. you don't collect patches
and *maintain* them on an upstream branch.  Granted some of you guys
are doing a great job of maintaining branches on your downstream or
BSP kernels, but conduct a Reviewer type role for upstream.

You guys are pushing back like this is some kind of demotion.  That's
not the case at all.  All it does is better describe the (very worthy)
function you *actually* provide.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Chanwoo Choi]

Hi Lee,

On 2015년 10월 28일 00:42, Lee Jones wrote:
> Since eafbaac ("MAINTAINERS: Add "R:" designated-reviewers tag") we
> have been able to tag specific people as Reviewers.  These are key
> individuals who are tasked with or volunteer to review code submitted
> to a subsystem or specific file.  However, according to MAINTAINERS
> we have 1046 Maintainers and only a mere 22 Reviewers.  I believe
> these numbers to be incorrect, as many of these Maintainers are in
> fact Reviewers.
> 
> I have taken the time to identify some of the Reviewers who pertain
> to subsystems which I look after, and have changed their status from
> Maintainer (collector of patches) to Reviewer (reviewer of code).
> 
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
> 
>  MAINTAINERS | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)

Although this patch don't include the entry of drivers/extcon,
I agree your opinion to add 'R' for Reviewer.

Acked-by: Chanwoo Choi <cw00.choi@samsung.com>

Thanks,
Chanwoo Choi

[PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Chanwoo Choi]

Hi Lee,

On 2015? 10? 28? 00:42, Lee Jones wrote:
> Since eafbaac ("MAINTAINERS: Add "R:" designated-reviewers tag") we
> have been able to tag specific people as Reviewers.  These are key
> individuals who are tasked with or volunteer to review code submitted
> to a subsystem or specific file.  However, according to MAINTAINERS
> we have 1046 Maintainers and only a mere 22 Reviewers.  I believe
> these numbers to be incorrect, as many of these Maintainers are in
> fact Reviewers.
> 
> I have taken the time to identify some of the Reviewers who pertain
> to subsystems which I look after, and have changed their status from
> Maintainer (collector of patches) to Reviewer (reviewer of code).
> 
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
> 
>  MAINTAINERS | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)

Although this patch don't include the entry of drivers/extcon,
I agree your opinion to add 'R' for Reviewer.

Acked-by: Chanwoo Choi <cw00.choi@samsung.com>

Thanks,
Chanwoo Choi

Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Javier Martinez Canillas]

Hello Lee,

On Wed, Oct 28, 2015 at 9:24 AM, Lee Jones <lee.jones@linaro.org> wrote:
> On Tue, 2015-10-27 at 18:15 +0000, Lee Jones wrote:
>> On Tue, 27 Oct 2015, Sebastian Reichel wrote:
>> > On Tue, Oct 27, 2015 at 03:42:37PM +0000, Lee Jones wrote:
>> > > Since eafbaac ("MAINTAINERS: Add "R:" designated-reviewers tag") we
>> > > have been able to tag specific people as Reviewers.  These are key
>> > > individuals who are tasked with or volunteer to review code submitted
>> > > to a subsystem or specific file.  However, according to MAINTAINERS
>> > > we have 1046 Maintainers and only a mere 22 Reviewers.  I believe
>> > > these numbers to be incorrect, as many of these Maintainers are in
>> > > fact Reviewers.
>
> Most entries in MAINTAINERS seem to be vanity entries than actual
> active participants.  A person typically writes a driver, adds a
> MAINTAINER entry, then forgets about it and/or the hardware becomes
> outdated.
>
> This I agree with.
>
> On Wed, 28 Oct 2015, Krzysztof Kozlowski wrote:
>> 2015-10-28 3:44 GMT+09:00 Joe Perches <joe@perches.com>:
>> > On Tue, 2015-10-27 at 18:15 +0000, Lee Jones wrote:
>> > > On Tue, 27 Oct 2015, Sebastian Reichel wrote:> >
>> > > > I think you should CC the people, which are changed from "M:" to
>> > > > "R:", though.
>> > >
>> > > Yes, makes sense.
>> > >
>> > > I'd like to collect some Maintainer Acks first though.
>> >
>> > I think people from organizations like Samsung are actual
>> > maintainers not reviewers.
>
> So this all hinges on how we are describing Maintainers and Reviewers.
>
> My personal definition (until convinced otherwise) is that Reviewers
> care about their particular subsystem and/or files.  They conduct code
> reviews to ensure nothing gets broken and the code base stays in best
> possible state of worthiness.  On the other hand Maintainers usually
> conduct themselves as Reviewers but also have 'maintainership' duties
> as well; such as applying patches, *maintaining*, testing, rebasing,
> etc, an upstream branch and ultimately sending pull-requests to higher
> level Maintainers i.e. Linus.  Maintainers also have the ultimate say
> (unless over-ruled by Linus etc) over what gets applied.
>
>> > Their drivers are not thrown over a wall and forgotten.
>>

I've a different definition. For me it depends on much do you care
about the component. For example I maintain a couple of drivers in the
kernel and Device Tree files for some boards that are important to me
but I also care about some other subsystems (i.e: Exynos SoC support)
and I act as a reviewer (although I'm not officially listed as
reviewer in the MAINTAINERS file).

We do have in fact different tags for each type of involvement so I
usually answer with a Reviewed-by tag if I review code for a subsystem
I care but I don't maintainer or answer with an Acked-by tag if I
review *and agree* with a patch for a component I maintain (so the
maintainer knows that is good to apply differently from the list if
needed).

Now, that doesn't mean that I provide a pull request for the drivers
or boards I maintain on every release since that will depend on the
number of patches posted for that component per release. So if there
are only a couple of patches, I think is easier for the subsystem
maintainer to pick those directly from the list but if there are a lot
of them, then the maintainer may ask me to prepare a branch to pull
and I've done in the past for drivers I maintain to be sure that the
patches in the list are applied in the right order, no needed patches
were missed, etc.

Another difference is that when I'm listed as a maintainer, I feel an
obligation to answer to the patches touching that component but that's
not the case for components I usually act as a reviewer, I may review
it if I have time but if I don't, I let other people to review it.

>> At least for Samsung Multifunction PMIC drivers (and some of Maxim
>> MUICs and PMICs) these are actively used by us in existing and new
>> products. They are also continuously extended and actually maintained.
>> This means that it is not only about review of new patches but also
>> about caring that nothing will become broken.
>
> Exactly.  This what I expect of any good code Reviewer.
>
>> I would prefer to leave the "SAMSUNG MULTIFUNCTION PMIC DEVICE
>> DRIVERS" entry as is - maintainers.
>

I agree with Krzysztof here, I would prefer to keep them as
maintainers if they are maintaining the drivers.

> But you aren't maintaining the driver i.e. you don't collect patches
> and *maintain* them on an upstream branch.  Granted some of you guys
> are doing a great job of maintaining branches on your downstream or
> BSP kernels, but conduct a Reviewer type role for upstream.
>
> You guys are pushing back like this is some kind of demotion.  That's
> not the case at all.  All it does is better describe the (very worthy)
> function you *actually* provide.
>

But I think it makes description less accurate in fact, since without
$SUBJECT get_maintainers.pl reports for example:

Krzysztof Kozlowski <k.kozlowski@samsung.com> (supporter:MAXIM PMIC
AND MUIC DRIVERS FOR EXYNOS BASED BO...)
Lee Jones <lee.jones@linaro.org> (supporter:MULTIFUNCTION DEVICES (MFD))

and after the change:

Krzysztof Kozlowski <k.kozlowski@samsung.com> (reviewer)
Lee Jones <lee.jones@linaro.org> (supporter:MULTIFUNCTION DEVICES (MFD))

He also works for Samsung so the driver is not only maintained but
supported since he can actually take care of it as a part of his day
job (if I understood correctly).

> --
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog
>

Best regards,
Javier

[PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Javier Martinez Canillas]

Hello Lee,

On Wed, Oct 28, 2015 at 9:24 AM, Lee Jones <lee.jones@linaro.org> wrote:
> On Tue, 2015-10-27 at 18:15 +0000, Lee Jones wrote:
>> On Tue, 27 Oct 2015, Sebastian Reichel wrote:
>> > On Tue, Oct 27, 2015 at 03:42:37PM +0000, Lee Jones wrote:
>> > > Since eafbaac ("MAINTAINERS: Add "R:" designated-reviewers tag") we
>> > > have been able to tag specific people as Reviewers.  These are key
>> > > individuals who are tasked with or volunteer to review code submitted
>> > > to a subsystem or specific file.  However, according to MAINTAINERS
>> > > we have 1046 Maintainers and only a mere 22 Reviewers.  I believe
>> > > these numbers to be incorrect, as many of these Maintainers are in
>> > > fact Reviewers.
>
> Most entries in MAINTAINERS seem to be vanity entries than actual
> active participants.  A person typically writes a driver, adds a
> MAINTAINER entry, then forgets about it and/or the hardware becomes
> outdated.
>
> This I agree with.
>
> On Wed, 28 Oct 2015, Krzysztof Kozlowski wrote:
>> 2015-10-28 3:44 GMT+09:00 Joe Perches <joe@perches.com>:
>> > On Tue, 2015-10-27 at 18:15 +0000, Lee Jones wrote:
>> > > On Tue, 27 Oct 2015, Sebastian Reichel wrote:> >
>> > > > I think you should CC the people, which are changed from "M:" to
>> > > > "R:", though.
>> > >
>> > > Yes, makes sense.
>> > >
>> > > I'd like to collect some Maintainer Acks first though.
>> >
>> > I think people from organizations like Samsung are actual
>> > maintainers not reviewers.
>
> So this all hinges on how we are describing Maintainers and Reviewers.
>
> My personal definition (until convinced otherwise) is that Reviewers
> care about their particular subsystem and/or files.  They conduct code
> reviews to ensure nothing gets broken and the code base stays in best
> possible state of worthiness.  On the other hand Maintainers usually
> conduct themselves as Reviewers but also have 'maintainership' duties
> as well; such as applying patches, *maintaining*, testing, rebasing,
> etc, an upstream branch and ultimately sending pull-requests to higher
> level Maintainers i.e. Linus.  Maintainers also have the ultimate say
> (unless over-ruled by Linus etc) over what gets applied.
>
>> > Their drivers are not thrown over a wall and forgotten.
>>

I've a different definition. For me it depends on much do you care
about the component. For example I maintain a couple of drivers in the
kernel and Device Tree files for some boards that are important to me
but I also care about some other subsystems (i.e: Exynos SoC support)
and I act as a reviewer (although I'm not officially listed as
reviewer in the MAINTAINERS file).

We do have in fact different tags for each type of involvement so I
usually answer with a Reviewed-by tag if I review code for a subsystem
I care but I don't maintainer or answer with an Acked-by tag if I
review *and agree* with a patch for a component I maintain (so the
maintainer knows that is good to apply differently from the list if
needed).

Now, that doesn't mean that I provide a pull request for the drivers
or boards I maintain on every release since that will depend on the
number of patches posted for that component per release. So if there
are only a couple of patches, I think is easier for the subsystem
maintainer to pick those directly from the list but if there are a lot
of them, then the maintainer may ask me to prepare a branch to pull
and I've done in the past for drivers I maintain to be sure that the
patches in the list are applied in the right order, no needed patches
were missed, etc.

Another difference is that when I'm listed as a maintainer, I feel an
obligation to answer to the patches touching that component but that's
not the case for components I usually act as a reviewer, I may review
it if I have time but if I don't, I let other people to review it.

>> At least for Samsung Multifunction PMIC drivers (and some of Maxim
>> MUICs and PMICs) these are actively used by us in existing and new
>> products. They are also continuously extended and actually maintained.
>> This means that it is not only about review of new patches but also
>> about caring that nothing will become broken.
>
> Exactly.  This what I expect of any good code Reviewer.
>
>> I would prefer to leave the "SAMSUNG MULTIFUNCTION PMIC DEVICE
>> DRIVERS" entry as is - maintainers.
>

I agree with Krzysztof here, I would prefer to keep them as
maintainers if they are maintaining the drivers.

> But you aren't maintaining the driver i.e. you don't collect patches
> and *maintain* them on an upstream branch.  Granted some of you guys
> are doing a great job of maintaining branches on your downstream or
> BSP kernels, but conduct a Reviewer type role for upstream.
>
> You guys are pushing back like this is some kind of demotion.  That's
> not the case at all.  All it does is better describe the (very worthy)
> function you *actually* provide.
>

But I think it makes description less accurate in fact, since without
$SUBJECT get_maintainers.pl reports for example:

Krzysztof Kozlowski <k.kozlowski@samsung.com> (supporter:MAXIM PMIC
AND MUIC DRIVERS FOR EXYNOS BASED BO...)
Lee Jones <lee.jones@linaro.org> (supporter:MULTIFUNCTION DEVICES (MFD))

and after the change:

Krzysztof Kozlowski <k.kozlowski@samsung.com> (reviewer)
Lee Jones <lee.jones@linaro.org> (supporter:MULTIFUNCTION DEVICES (MFD))

He also works for Samsung so the driver is not only maintained but
supported since he can actually take care of it as a part of his day
job (if I understood correctly).

> --
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead
> Linaro.org ? Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog
>

Best regards,
Javier

Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Krzysztof Kozlowski]

On 28.10.2015 17:24, Lee Jones wrote:
> On Wed, 28 Oct 2015, Krzysztof Kozlowski wrote:
>> 2015-10-28 3:44 GMT+09:00 Joe Perches <joe@perches.com>:
>>> On Tue, 2015-10-27 at 18:15 +0000, Lee Jones wrote:
>>>> On Tue, 27 Oct 2015, Sebastian Reichel wrote:> >
>>>>> I think you should CC the people, which are changed from "M:"
>>>>> to "R:", though.
>>>> 
>>>> Yes, makes sense.
>>>> 
>>>> I'd like to collect some Maintainer Acks first though.
>>> 
>>> I think people from organizations like Samsung are actual 
>>> maintainers not reviewers.
> 
> So this all hinges on how we are describing Maintainers and
> Reviewers.
> 
> My personal definition (until convinced otherwise) is that Reviewers 
> care about their particular subsystem and/or files.  They conduct
> code reviews to ensure nothing gets broken and the code base stays in
> best possible state of worthiness.  On the other hand Maintainers
> usually conduct themselves as Reviewers but also have
> 'maintainership' duties as well; such as applying patches,
> *maintaining*, testing, rebasing, etc, an upstream branch and
> ultimately sending pull-requests to higher level Maintainers i.e.
> Linus.  Maintainers also have the ultimate say (unless over-ruled by
> Linus etc) over what gets applied.

Okay, sounds reasonable... so if a person performs reviews plus he does
some of the other activities (not all) then who is he?

For example reviewing + testing + fixing bugs + cleaning up (sending
patches from time to time)?

Would that be sufficient requirement to call him maintainer of a driver?
Or maybe all of these requirements must be met (including handling of
patches and sending pull reqs)?


>>> Their drivers are not thrown over a wall and forgotten.
>> 
>> At least for Samsung Multifunction PMIC drivers (and some of Maxim 
>> MUICs and PMICs) these are actively used by us in existing and new 
>> products. They are also continuously extended and actually
>> maintained. This means that it is not only about review of new
>> patches but also about caring that nothing will become broken.
> 
> Exactly.  This what I expect of any good code Reviewer.
> 
>> I would prefer to leave the "SAMSUNG MULTIFUNCTION PMIC DEVICE 
>> DRIVERS" entry as is - maintainers.
> 
> But you aren't maintaining the driver i.e. you don't collect patches 
> and *maintain* them on an upstream branch.

Indeed, we don't. However are other non-reviewing activities sufficient?

> Granted some of you guys 
> are doing a great job of maintaining branches on your downstream or 
> BSP kernels, but conduct a Reviewer type role for upstream.

You mentioned also the "ultimate say over what gets applied" - which in
this particular case is interesting to us because we have direct
interest in these drivers being in a good shape and doing things we
expect them to do. Like representing the interest of users.

Of course one could say that every upstreaming person has such
expectations... but some of the upstreamers just send a driver for one
device. Or extend driver for one device. In this case this is a family
of devices used on all of our Exynos SoC products and we care about all
of them.


> You guys are pushing back like this is some kind of demotion.
> That's not the case at all.  All it does is better describe the (very
> worthy) function you *actually* provide.

It is getting into dispute about entire change of yours... which is not
what I want. I agree with your general idea but I was referring only to
that particular case - the Samsung PMICs (and Maxim PMICs/MUICs which
would fall into same category).

Best regards,
Krzysztof

[PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Krzysztof Kozlowski]

On 28.10.2015 17:24, Lee Jones wrote:
> On Wed, 28 Oct 2015, Krzysztof Kozlowski wrote:
>> 2015-10-28 3:44 GMT+09:00 Joe Perches <joe@perches.com>:
>>> On Tue, 2015-10-27 at 18:15 +0000, Lee Jones wrote:
>>>> On Tue, 27 Oct 2015, Sebastian Reichel wrote:> >
>>>>> I think you should CC the people, which are changed from "M:"
>>>>> to "R:", though.
>>>> 
>>>> Yes, makes sense.
>>>> 
>>>> I'd like to collect some Maintainer Acks first though.
>>> 
>>> I think people from organizations like Samsung are actual 
>>> maintainers not reviewers.
> 
> So this all hinges on how we are describing Maintainers and
> Reviewers.
> 
> My personal definition (until convinced otherwise) is that Reviewers 
> care about their particular subsystem and/or files.  They conduct
> code reviews to ensure nothing gets broken and the code base stays in
> best possible state of worthiness.  On the other hand Maintainers
> usually conduct themselves as Reviewers but also have
> 'maintainership' duties as well; such as applying patches,
> *maintaining*, testing, rebasing, etc, an upstream branch and
> ultimately sending pull-requests to higher level Maintainers i.e.
> Linus.  Maintainers also have the ultimate say (unless over-ruled by
> Linus etc) over what gets applied.

Okay, sounds reasonable... so if a person performs reviews plus he does
some of the other activities (not all) then who is he?

For example reviewing + testing + fixing bugs + cleaning up (sending
patches from time to time)?

Would that be sufficient requirement to call him maintainer of a driver?
Or maybe all of these requirements must be met (including handling of
patches and sending pull reqs)?


>>> Their drivers are not thrown over a wall and forgotten.
>> 
>> At least for Samsung Multifunction PMIC drivers (and some of Maxim 
>> MUICs and PMICs) these are actively used by us in existing and new 
>> products. They are also continuously extended and actually
>> maintained. This means that it is not only about review of new
>> patches but also about caring that nothing will become broken.
> 
> Exactly.  This what I expect of any good code Reviewer.
> 
>> I would prefer to leave the "SAMSUNG MULTIFUNCTION PMIC DEVICE 
>> DRIVERS" entry as is - maintainers.
> 
> But you aren't maintaining the driver i.e. you don't collect patches 
> and *maintain* them on an upstream branch.

Indeed, we don't. However are other non-reviewing activities sufficient?

> Granted some of you guys 
> are doing a great job of maintaining branches on your downstream or 
> BSP kernels, but conduct a Reviewer type role for upstream.

You mentioned also the "ultimate say over what gets applied" - which in
this particular case is interesting to us because we have direct
interest in these drivers being in a good shape and doing things we
expect them to do. Like representing the interest of users.

Of course one could say that every upstreaming person has such
expectations... but some of the upstreamers just send a driver for one
device. Or extend driver for one device. In this case this is a family
of devices used on all of our Exynos SoC products and we care about all
of them.


> You guys are pushing back like this is some kind of demotion.
> That's not the case at all.  All it does is better describe the (very
> worthy) function you *actually* provide.

It is getting into dispute about entire change of yours... which is not
what I want. I agree with your general idea but I was referring only to
that particular case - the Samsung PMICs (and Maxim PMICs/MUICs which
would fall into same category).

Best regards,
Krzysztof

Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Javier Martinez Canillas]

On Wed, Oct 28, 2015 at 10:21 AM, Javier Martinez Canillas
<javier@dowhile0.org> wrote:
>
> We do have in fact different tags for each type of involvement so I
> usually answer with a Reviewed-by tag if I review code for a subsystem
> I care but I don't maintainer or answer with an Acked-by tag if I
> review *and agree* with a patch for a component I maintain (so the
> maintainer knows that is good to apply differently from the list if

I wanted to write directly instead of differently. Sorry about the
typo, I'm still with jetlag after a long trip.

Best regards,
Javier

[PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Javier Martinez Canillas]

On Wed, Oct 28, 2015 at 10:21 AM, Javier Martinez Canillas
<javier@dowhile0.org> wrote:
>
> We do have in fact different tags for each type of involvement so I
> usually answer with a Reviewed-by tag if I review code for a subsystem
> I care but I don't maintainer or answer with an Acked-by tag if I
> review *and agree* with a patch for a component I maintain (so the
> maintainer knows that is good to apply differently from the list if

I wanted to write directly instead of differently. Sorry about the
typo, I'm still with jetlag after a long trip.

Best regards,
Javier

Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Krzysztof Kozlowski]

On 28.10.2015 18:21, Javier Martinez Canillas wrote:
> Hello Lee,
> 

(...)
Let me only add something to certain part of your email...

>> But you aren't maintaining the driver i.e. you don't collect patches
>> and *maintain* them on an upstream branch.  Granted some of you guys
>> are doing a great job of maintaining branches on your downstream or
>> BSP kernels, but conduct a Reviewer type role for upstream.
>>
>> You guys are pushing back like this is some kind of demotion.  That's
>> not the case at all.  All it does is better describe the (very worthy)
>> function you *actually* provide.
>>
> 
> But I think it makes description less accurate in fact, since without
> $SUBJECT get_maintainers.pl reports for example:
> 
> Krzysztof Kozlowski <k.kozlowski@samsung.com> (supporter:MAXIM PMIC
> AND MUIC DRIVERS FOR EXYNOS BASED BO...)
> Lee Jones <lee.jones@linaro.org> (supporter:MULTIFUNCTION DEVICES (MFD))
> 
> and after the change:
> 
> Krzysztof Kozlowski <k.kozlowski@samsung.com> (reviewer)
> Lee Jones <lee.jones@linaro.org> (supporter:MULTIFUNCTION DEVICES (MFD))
> 
> He also works for Samsung so the driver is not only maintained but
> supported since he can actually take care of it as a part of his day
> job (if I understood correctly).

Oh, that's interesting semantic change. Yes, in that particular case, I
added the "supported" tag on purpose - it's part of my job. It is
connected with what I said in other reply - we have deep interest in
these drivers. Not only "I will review the code if I have the time". No.
I will devote my time to ensure that the code is working on our products.

Best regards,
Krzysztof

[PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Krzysztof Kozlowski]

On 28.10.2015 18:21, Javier Martinez Canillas wrote:
> Hello Lee,
> 

(...)
Let me only add something to certain part of your email...

>> But you aren't maintaining the driver i.e. you don't collect patches
>> and *maintain* them on an upstream branch.  Granted some of you guys
>> are doing a great job of maintaining branches on your downstream or
>> BSP kernels, but conduct a Reviewer type role for upstream.
>>
>> You guys are pushing back like this is some kind of demotion.  That's
>> not the case at all.  All it does is better describe the (very worthy)
>> function you *actually* provide.
>>
> 
> But I think it makes description less accurate in fact, since without
> $SUBJECT get_maintainers.pl reports for example:
> 
> Krzysztof Kozlowski <k.kozlowski@samsung.com> (supporter:MAXIM PMIC
> AND MUIC DRIVERS FOR EXYNOS BASED BO...)
> Lee Jones <lee.jones@linaro.org> (supporter:MULTIFUNCTION DEVICES (MFD))
> 
> and after the change:
> 
> Krzysztof Kozlowski <k.kozlowski@samsung.com> (reviewer)
> Lee Jones <lee.jones@linaro.org> (supporter:MULTIFUNCTION DEVICES (MFD))
> 
> He also works for Samsung so the driver is not only maintained but
> supported since he can actually take care of it as a part of his day
> job (if I understood correctly).

Oh, that's interesting semantic change. Yes, in that particular case, I
added the "supported" tag on purpose - it's part of my job. It is
connected with what I said in other reply - we have deep interest in
these drivers. Not only "I will review the code if I have the time". No.
I will devote my time to ensure that the code is working on our products.

Best regards,
Krzysztof

Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Uwe Kleine-König]

Hello,

On Wed, Oct 28, 2015 at 06:23:37PM +0900, Krzysztof Kozlowski wrote:
> On 28.10.2015 17:24, Lee Jones wrote:
> > You guys are pushing back like this is some kind of demotion.
> > That's not the case at all.  All it does is better describe the (very
> > worthy) function you *actually* provide.
> 
> It is getting into dispute about entire change of yours... which is not
> what I want. I agree with your general idea but I was referring only to
> that particular case - the Samsung PMICs (and Maxim PMICs/MUICs which
> would fall into same category).

Not being affected by this change, I wonder what the technical
difference is if someone is listed as reviewer instead of maintainer.
Does get_maintainer.pl behave differently?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Uwe Kleine-König]

Hello,

On Wed, Oct 28, 2015 at 06:23:37PM +0900, Krzysztof Kozlowski wrote:
> On 28.10.2015 17:24, Lee Jones wrote:
> > You guys are pushing back like this is some kind of demotion.
> > That's not the case at all.  All it does is better describe the (very
> > worthy) function you *actually* provide.
> 
> It is getting into dispute about entire change of yours... which is not
> what I want. I agree with your general idea but I was referring only to
> that particular case - the Samsung PMICs (and Maxim PMICs/MUICs which
> would fall into same category).

Not being affected by this change, I wonder what the technical
difference is if someone is listed as reviewer instead of maintainer.
Does get_maintainer.pl behave differently?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Lee Jones]

On Wed, 28 Oct 2015, Uwe Kleine-König wrote:

> Hello,
> 
> On Wed, Oct 28, 2015 at 06:23:37PM +0900, Krzysztof Kozlowski wrote:
> > On 28.10.2015 17:24, Lee Jones wrote:
> > > You guys are pushing back like this is some kind of demotion.
> > > That's not the case at all.  All it does is better describe the (very
> > > worthy) function you *actually* provide.
> > 
> > It is getting into dispute about entire change of yours... which is not
> > what I want. I agree with your general idea but I was referring only to
> > that particular case - the Samsung PMICs (and Maxim PMICs/MUICs which
> > would fall into same category).
> 
> Not being affected by this change, I wonder what the technical
> difference is if someone is listed as reviewer instead of maintainer.
> Does get_maintainer.pl behave differently?

Reviewers are people who should be Cc'ed on all patches, as are
Maintainers, so no, fundamentally they are treated the same by a
Submitter.

Here is some output for Maintainers/Reviews from get_maintainer.pl.

 $ ./scripts/get_maintainer.pl -f arch/arm/boot/dts/vf500.dtsi
 Shawn Guo <shawnguo@kernel.org> (maintainer:ARM/FREESCALE VYBRID ARM ARCHITECTURE)
 Sascha Hauer <kernel@pengutronix.de> (maintainer:ARM/FREESCALE VYBRID ARM ARCHITECTURE)
 Stefan Agner <stefan@agner.ch> (reviewer)
 Russell King <linux@arm.linux.org.uk> (maintainer:ARM PORT)
 linux-arm-kernel@lists.infradead.org (moderated list:ARM/FREESCALE VYBRID ARM ARCHITECTURE)
 devicetree@vger.kernel.org (open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS)
 linux-kernel@vger.kernel.org (open list)

I'm sure we can make the output even more similar by listing the
MAINTAINERS tag after "reviewer" too.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Lee Jones]

On Wed, 28 Oct 2015, Uwe Kleine-K?nig wrote:

> Hello,
> 
> On Wed, Oct 28, 2015 at 06:23:37PM +0900, Krzysztof Kozlowski wrote:
> > On 28.10.2015 17:24, Lee Jones wrote:
> > > You guys are pushing back like this is some kind of demotion.
> > > That's not the case at all.  All it does is better describe the (very
> > > worthy) function you *actually* provide.
> > 
> > It is getting into dispute about entire change of yours... which is not
> > what I want. I agree with your general idea but I was referring only to
> > that particular case - the Samsung PMICs (and Maxim PMICs/MUICs which
> > would fall into same category).
> 
> Not being affected by this change, I wonder what the technical
> difference is if someone is listed as reviewer instead of maintainer.
> Does get_maintainer.pl behave differently?

Reviewers are people who should be Cc'ed on all patches, as are
Maintainers, so no, fundamentally they are treated the same by a
Submitter.

Here is some output for Maintainers/Reviews from get_maintainer.pl.

 $ ./scripts/get_maintainer.pl -f arch/arm/boot/dts/vf500.dtsi
 Shawn Guo <shawnguo@kernel.org> (maintainer:ARM/FREESCALE VYBRID ARM ARCHITECTURE)
 Sascha Hauer <kernel@pengutronix.de> (maintainer:ARM/FREESCALE VYBRID ARM ARCHITECTURE)
 Stefan Agner <stefan@agner.ch> (reviewer)
 Russell King <linux@arm.linux.org.uk> (maintainer:ARM PORT)
 linux-arm-kernel at lists.infradead.org (moderated list:ARM/FREESCALE VYBRID ARM ARCHITECTURE)
 devicetree at vger.kernel.org (open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS)
 linux-kernel at vger.kernel.org (open list)

I'm sure we can make the output even more similar by listing the
MAINTAINERS tag after "reviewer" too.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Lee Jones]

On Wed, 28 Oct 2015, Krzysztof Kozlowski wrote:
> On 28.10.2015 17:24, Lee Jones wrote:
> > On Wed, 28 Oct 2015, Krzysztof Kozlowski wrote:
> >> 2015-10-28 3:44 GMT+09:00 Joe Perches <joe@perches.com>:
> >>> On Tue, 2015-10-27 at 18:15 +0000, Lee Jones wrote:
> >>>> On Tue, 27 Oct 2015, Sebastian Reichel wrote:> >
> >>>>> I think you should CC the people, which are changed from "M:"
> >>>>> to "R:", though.
> >>>> 
> >>>> Yes, makes sense.
> >>>> 
> >>>> I'd like to collect some Maintainer Acks first though.
> >>> 
> >>> I think people from organizations like Samsung are actual 
> >>> maintainers not reviewers.
> > 
> > So this all hinges on how we are describing Maintainers and
> > Reviewers.
> > 
> > My personal definition (until convinced otherwise) is that Reviewers 
> > care about their particular subsystem and/or files.  They conduct
> > code reviews to ensure nothing gets broken and the code base stays in
> > best possible state of worthiness.  On the other hand Maintainers
> > usually conduct themselves as Reviewers but also have
> > 'maintainership' duties as well; such as applying patches,
> > *maintaining*, testing, rebasing, etc, an upstream branch and
> > ultimately sending pull-requests to higher level Maintainers i.e.
> > Linus.  Maintainers also have the ultimate say (unless over-ruled by
> > Linus etc) over what gets applied.
> 
> Okay, sounds reasonable... so if a person performs reviews plus he does
> some of the other activities (not all) then who is he?

LT;DR: If someone doesn't *maintain* an upstream branch, they are not
an upstream Maintainer.

> For example reviewing

Depends on the type of engagement.  Anyone can review any patch
submitted to anywhere on the code-base.  This does not make them an
accepted Reviewer.  Here I'm saying that a Reviewer is a competent
engineer who has made a promise to dedicate time to review incoming
patches in order to ensure quality.

To emphasise a tagged Reviewer isn't just someone who reviews code
every now and again.  It's someone who cares and has a vested interest
in either a subsystem as a whole, or perhaps individual or a group of
drivers.  However, this person does not conduct upstream branch
*maintenance*.

> testing + fixing bugs + cleaning up (sending
> patches from time to time)?

This is a Submitter/Contributor.

When I said testing before, I meant the branch being maintained, not
the driver on it's own.  That should be tested by Submitters/
Contributors or Testers (who get to provide their Tested-by).

> Would that be sufficient requirement to call him maintainer of a driver?
> Or maybe all of these requirements must be met (including handling of
> patches and sending pull reqs)?

It's only really the handing of patches and the maintenance of an
upstream branch which differentiates a Reviewer from a Maintainer. 

> >>> Their drivers are not thrown over a wall and forgotten.
> >> 
> >> At least for Samsung Multifunction PMIC drivers (and some of Maxim 
> >> MUICs and PMICs) these are actively used by us in existing and new 
> >> products. They are also continuously extended and actually
> >> maintained. This means that it is not only about review of new
> >> patches but also about caring that nothing will become broken.
> > 
> > Exactly.  This what I expect of any good code Reviewer.
> > 
> >> I would prefer to leave the "SAMSUNG MULTIFUNCTION PMIC DEVICE 
> >> DRIVERS" entry as is - maintainers.
> > 
> > But you aren't maintaining the driver i.e. you don't collect patches 
> > and *maintain* them on an upstream branch.
> 
> Indeed, we don't. However are other non-reviewing activities sufficient?

The other non-reviewing activities you mentioned are that of the
Submitter/Contributor/Tester.  They still don't make someone a
Maintainer.

> > Granted some of you guys 
> > are doing a great job of maintaining branches on your downstream or 
> > BSP kernels, but conduct a Reviewer type role for upstream.
> 
> You mentioned also the "ultimate say over what gets applied" - which in
> this particular case is interesting to us because we have direct
> interest in these drivers being in a good shape and doing things we
> expect them to do. Like representing the interest of users.

Any Reviewers opinion will matter to someone who ultimately applies
the patches.  For instance, if you were to say to me "this change to
our MFD doesn't suit us because of X", I almost certainly won't apply
the patch.

> Of course one could say that every upstreaming person has such
> expectations... but some of the upstreamers just send a driver for one
> device. Or extend driver for one device. In this case this is a family
> of devices used on all of our Exynos SoC products and we care about all
> of them.

And being a nominated Reviewer mentioned in MAINTAINERS, that's
exactly what I'd expect.  If people fail to mean these requirement
they should be removed completely.

> > You guys are pushing back like this is some kind of demotion.
> > That's not the case at all.  All it does is better describe the (very
> > worthy) function you *actually* provide.
> 
> It is getting into dispute about entire change of yours... which is not
> what I want. I agree with your general idea but I was referring only to
> that particular case - the Samsung PMICs (and Maxim PMICs/MUICs which
> would fall into same category).

I hope that I've explained my view adequately above.  My view is also
carried out over the Samsung drivers.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Lee Jones]

On Wed, 28 Oct 2015, Krzysztof Kozlowski wrote:
> On 28.10.2015 17:24, Lee Jones wrote:
> > On Wed, 28 Oct 2015, Krzysztof Kozlowski wrote:
> >> 2015-10-28 3:44 GMT+09:00 Joe Perches <joe@perches.com>:
> >>> On Tue, 2015-10-27 at 18:15 +0000, Lee Jones wrote:
> >>>> On Tue, 27 Oct 2015, Sebastian Reichel wrote:> >
> >>>>> I think you should CC the people, which are changed from "M:"
> >>>>> to "R:", though.
> >>>> 
> >>>> Yes, makes sense.
> >>>> 
> >>>> I'd like to collect some Maintainer Acks first though.
> >>> 
> >>> I think people from organizations like Samsung are actual 
> >>> maintainers not reviewers.
> > 
> > So this all hinges on how we are describing Maintainers and
> > Reviewers.
> > 
> > My personal definition (until convinced otherwise) is that Reviewers 
> > care about their particular subsystem and/or files.  They conduct
> > code reviews to ensure nothing gets broken and the code base stays in
> > best possible state of worthiness.  On the other hand Maintainers
> > usually conduct themselves as Reviewers but also have
> > 'maintainership' duties as well; such as applying patches,
> > *maintaining*, testing, rebasing, etc, an upstream branch and
> > ultimately sending pull-requests to higher level Maintainers i.e.
> > Linus.  Maintainers also have the ultimate say (unless over-ruled by
> > Linus etc) over what gets applied.
> 
> Okay, sounds reasonable... so if a person performs reviews plus he does
> some of the other activities (not all) then who is he?

LT;DR: If someone doesn't *maintain* an upstream branch, they are not
an upstream Maintainer.

> For example reviewing

Depends on the type of engagement.  Anyone can review any patch
submitted to anywhere on the code-base.  This does not make them an
accepted Reviewer.  Here I'm saying that a Reviewer is a competent
engineer who has made a promise to dedicate time to review incoming
patches in order to ensure quality.

To emphasise a tagged Reviewer isn't just someone who reviews code
every now and again.  It's someone who cares and has a vested interest
in either a subsystem as a whole, or perhaps individual or a group of
drivers.  However, this person does not conduct upstream branch
*maintenance*.

> testing + fixing bugs + cleaning up (sending
> patches from time to time)?

This is a Submitter/Contributor.

When I said testing before, I meant the branch being maintained, not
the driver on it's own.  That should be tested by Submitters/
Contributors or Testers (who get to provide their Tested-by).

> Would that be sufficient requirement to call him maintainer of a driver?
> Or maybe all of these requirements must be met (including handling of
> patches and sending pull reqs)?

It's only really the handing of patches and the maintenance of an
upstream branch which differentiates a Reviewer from a Maintainer. 

> >>> Their drivers are not thrown over a wall and forgotten.
> >> 
> >> At least for Samsung Multifunction PMIC drivers (and some of Maxim 
> >> MUICs and PMICs) these are actively used by us in existing and new 
> >> products. They are also continuously extended and actually
> >> maintained. This means that it is not only about review of new
> >> patches but also about caring that nothing will become broken.
> > 
> > Exactly.  This what I expect of any good code Reviewer.
> > 
> >> I would prefer to leave the "SAMSUNG MULTIFUNCTION PMIC DEVICE 
> >> DRIVERS" entry as is - maintainers.
> > 
> > But you aren't maintaining the driver i.e. you don't collect patches 
> > and *maintain* them on an upstream branch.
> 
> Indeed, we don't. However are other non-reviewing activities sufficient?

The other non-reviewing activities you mentioned are that of the
Submitter/Contributor/Tester.  They still don't make someone a
Maintainer.

> > Granted some of you guys 
> > are doing a great job of maintaining branches on your downstream or 
> > BSP kernels, but conduct a Reviewer type role for upstream.
> 
> You mentioned also the "ultimate say over what gets applied" - which in
> this particular case is interesting to us because we have direct
> interest in these drivers being in a good shape and doing things we
> expect them to do. Like representing the interest of users.

Any Reviewers opinion will matter to someone who ultimately applies
the patches.  For instance, if you were to say to me "this change to
our MFD doesn't suit us because of X", I almost certainly won't apply
the patch.

> Of course one could say that every upstreaming person has such
> expectations... but some of the upstreamers just send a driver for one
> device. Or extend driver for one device. In this case this is a family
> of devices used on all of our Exynos SoC products and we care about all
> of them.

And being a nominated Reviewer mentioned in MAINTAINERS, that's
exactly what I'd expect.  If people fail to mean these requirement
they should be removed completely.

> > You guys are pushing back like this is some kind of demotion.
> > That's not the case at all.  All it does is better describe the (very
> > worthy) function you *actually* provide.
> 
> It is getting into dispute about entire change of yours... which is not
> what I want. I agree with your general idea but I was referring only to
> that particular case - the Samsung PMICs (and Maxim PMICs/MUICs which
> would fall into same category).

I hope that I've explained my view adequately above.  My view is also
carried out over the Samsung drivers.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Lee Jones]

On Wed, 28 Oct 2015, Javier Martinez Canillas wrote:
> On Wed, Oct 28, 2015 at 9:24 AM, Lee Jones <lee.jones@linaro.org> wrote:
> > On Tue, 2015-10-27 at 18:15 +0000, Lee Jones wrote:
> >> On Tue, 27 Oct 2015, Sebastian Reichel wrote:
> >> > On Tue, Oct 27, 2015 at 03:42:37PM +0000, Lee Jones wrote:
> >> > > Since eafbaac ("MAINTAINERS: Add "R:" designated-reviewers tag") we
> >> > > have been able to tag specific people as Reviewers.  These are key
> >> > > individuals who are tasked with or volunteer to review code submitted
> >> > > to a subsystem or specific file.  However, according to MAINTAINERS
> >> > > we have 1046 Maintainers and only a mere 22 Reviewers.  I believe
> >> > > these numbers to be incorrect, as many of these Maintainers are in
> >> > > fact Reviewers.
> >
> > Most entries in MAINTAINERS seem to be vanity entries than actual
> > active participants.  A person typically writes a driver, adds a
> > MAINTAINER entry, then forgets about it and/or the hardware becomes
> > outdated.
> >
> > This I agree with.
> >
> > On Wed, 28 Oct 2015, Krzysztof Kozlowski wrote:
> >> 2015-10-28 3:44 GMT+09:00 Joe Perches <joe@perches.com>:
> >> > On Tue, 2015-10-27 at 18:15 +0000, Lee Jones wrote:
> >> > > On Tue, 27 Oct 2015, Sebastian Reichel wrote:> >
> >> > > > I think you should CC the people, which are changed from "M:" to
> >> > > > "R:", though.
> >> > >
> >> > > Yes, makes sense.
> >> > >
> >> > > I'd like to collect some Maintainer Acks first though.
> >> >
> >> > I think people from organizations like Samsung are actual
> >> > maintainers not reviewers.
> >
> > So this all hinges on how we are describing Maintainers and Reviewers.
> >
> > My personal definition (until convinced otherwise) is that Reviewers
> > care about their particular subsystem and/or files.  They conduct code
> > reviews to ensure nothing gets broken and the code base stays in best
> > possible state of worthiness.  On the other hand Maintainers usually
> > conduct themselves as Reviewers but also have 'maintainership' duties
> > as well; such as applying patches, *maintaining*, testing, rebasing,
> > etc, an upstream branch and ultimately sending pull-requests to higher
> > level Maintainers i.e. Linus.  Maintainers also have the ultimate say
> > (unless over-ruled by Linus etc) over what gets applied.
> >
> >> > Their drivers are not thrown over a wall and forgotten.
> >>
> 
> I've a different definition. For me it depends on much do you care
> about the component. For example I maintain a couple of drivers in the
> kernel and Device Tree files for some boards that are important to me
> but I also care about some other subsystems (i.e: Exynos SoC support)
> and I act as a reviewer (although I'm not officially listed as
> reviewer in the MAINTAINERS file).

I wish to make this clear from the out-set.  If you have no obligation
to review patches but do so occasionally anyway, that does not make
you the type of Reviewer that we're speaking about here.  Anyone can
review any patch on the list that they wish to, which is lovely, but
it won't carry the same authority (for want of a better expression) as
if you were tagged as an official Reviewer in MAINTAINERS.

> We do have in fact different tags for each type of involvement so I
> usually answer with a Reviewed-by tag if I review code for a subsystem
> I care but I don't maintainer or answer with an Acked-by tag if I
> review *and agree* with a patch for a component I maintain (so the
> maintainer knows that is good to apply differently from the list if
> needed).

I think you need to re-read what those tags mean.

  Documentation/SubmittingPatches

> Now, that doesn't mean that I provide a pull request for the drivers
> or boards I maintain on every release since that will depend on the
> number of patches posted for that component per release. So if there
> are only a couple of patches, I think is easier for the subsystem
> maintainer to pick those directly from the list but if there are a lot
> of them, then the maintainer may ask me to prepare a branch to pull
> and I've done in the past for drivers I maintain to be sure that the
> patches in the list are applied in the right order, no needed patches
> were missed, etc.

I have also submitted patches via a pull-request as a Submitter to
different subsystems.  That does not mean I should automatically be
classed as a Maintainer.

> Another difference is that when I'm listed as a maintainer, I feel an
> obligation to answer to the patches touching that component but that's
> not the case for components I usually act as a reviewer, I may review
> it if I have time but if I don't, I let other people to review it.

Then, in the latter case you shouldn't be listed as a Reviewer in my
example.  Anyone listed as a Reviewer in MAINTAINERS *does* have that
obligation.  That's what it means.  If Reviewers don't review, they
should be removed from MAINTAINERS.

> >> At least for Samsung Multifunction PMIC drivers (and some of Maxim
> >> MUICs and PMICs) these are actively used by us in existing and new
> >> products. They are also continuously extended and actually maintained.
> >> This means that it is not only about review of new patches but also
> >> about caring that nothing will become broken.
> >
> > Exactly.  This what I expect of any good code Reviewer.
> >
> >> I would prefer to leave the "SAMSUNG MULTIFUNCTION PMIC DEVICE
> >> DRIVERS" entry as is - maintainers.
> 
> I agree with Krzysztof here, I would prefer to keep them as
> maintainers if they are maintaining the drivers.

But they're not.  They're reviewing and caring like a good Reviewer
should.

> > But you aren't maintaining the driver i.e. you don't collect patches
> > and *maintain* them on an upstream branch.  Granted some of you guys
> > are doing a great job of maintaining branches on your downstream or
> > BSP kernels, but conduct a Reviewer type role for upstream.
> >
> > You guys are pushing back like this is some kind of demotion.  That's
> > not the case at all.  All it does is better describe the (very worthy)
> > function you *actually* provide.
> >
> 
> But I think it makes description less accurate in fact, since without
> $SUBJECT get_maintainers.pl reports for example:
> 
> Krzysztof Kozlowski <k.kozlowski@samsung.com> (supporter:MAXIM PMIC
> AND MUIC DRIVERS FOR EXYNOS BASED BO...)
> Lee Jones <lee.jones@linaro.org> (supporter:MULTIFUNCTION DEVICES (MFD))
> 
> and after the change:
> 
> Krzysztof Kozlowski <k.kozlowski@samsung.com> (reviewer)
> Lee Jones <lee.jones@linaro.org> (supporter:MULTIFUNCTION DEVICES (MFD))
> 
> He also works for Samsung so the driver is not only maintained but
> supported since he can actually take care of it as a part of his day
> job (if I understood correctly).

It's not the person that's supported, it's the driver.  The driver
doesn't need to change state.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Lee Jones]

On Wed, 28 Oct 2015, Javier Martinez Canillas wrote:
> On Wed, Oct 28, 2015 at 9:24 AM, Lee Jones <lee.jones@linaro.org> wrote:
> > On Tue, 2015-10-27 at 18:15 +0000, Lee Jones wrote:
> >> On Tue, 27 Oct 2015, Sebastian Reichel wrote:
> >> > On Tue, Oct 27, 2015 at 03:42:37PM +0000, Lee Jones wrote:
> >> > > Since eafbaac ("MAINTAINERS: Add "R:" designated-reviewers tag") we
> >> > > have been able to tag specific people as Reviewers.  These are key
> >> > > individuals who are tasked with or volunteer to review code submitted
> >> > > to a subsystem or specific file.  However, according to MAINTAINERS
> >> > > we have 1046 Maintainers and only a mere 22 Reviewers.  I believe
> >> > > these numbers to be incorrect, as many of these Maintainers are in
> >> > > fact Reviewers.
> >
> > Most entries in MAINTAINERS seem to be vanity entries than actual
> > active participants.  A person typically writes a driver, adds a
> > MAINTAINER entry, then forgets about it and/or the hardware becomes
> > outdated.
> >
> > This I agree with.
> >
> > On Wed, 28 Oct 2015, Krzysztof Kozlowski wrote:
> >> 2015-10-28 3:44 GMT+09:00 Joe Perches <joe@perches.com>:
> >> > On Tue, 2015-10-27 at 18:15 +0000, Lee Jones wrote:
> >> > > On Tue, 27 Oct 2015, Sebastian Reichel wrote:> >
> >> > > > I think you should CC the people, which are changed from "M:" to
> >> > > > "R:", though.
> >> > >
> >> > > Yes, makes sense.
> >> > >
> >> > > I'd like to collect some Maintainer Acks first though.
> >> >
> >> > I think people from organizations like Samsung are actual
> >> > maintainers not reviewers.
> >
> > So this all hinges on how we are describing Maintainers and Reviewers.
> >
> > My personal definition (until convinced otherwise) is that Reviewers
> > care about their particular subsystem and/or files.  They conduct code
> > reviews to ensure nothing gets broken and the code base stays in best
> > possible state of worthiness.  On the other hand Maintainers usually
> > conduct themselves as Reviewers but also have 'maintainership' duties
> > as well; such as applying patches, *maintaining*, testing, rebasing,
> > etc, an upstream branch and ultimately sending pull-requests to higher
> > level Maintainers i.e. Linus.  Maintainers also have the ultimate say
> > (unless over-ruled by Linus etc) over what gets applied.
> >
> >> > Their drivers are not thrown over a wall and forgotten.
> >>
> 
> I've a different definition. For me it depends on much do you care
> about the component. For example I maintain a couple of drivers in the
> kernel and Device Tree files for some boards that are important to me
> but I also care about some other subsystems (i.e: Exynos SoC support)
> and I act as a reviewer (although I'm not officially listed as
> reviewer in the MAINTAINERS file).

I wish to make this clear from the out-set.  If you have no obligation
to review patches but do so occasionally anyway, that does not make
you the type of Reviewer that we're speaking about here.  Anyone can
review any patch on the list that they wish to, which is lovely, but
it won't carry the same authority (for want of a better expression) as
if you were tagged as an official Reviewer in MAINTAINERS.

> We do have in fact different tags for each type of involvement so I
> usually answer with a Reviewed-by tag if I review code for a subsystem
> I care but I don't maintainer or answer with an Acked-by tag if I
> review *and agree* with a patch for a component I maintain (so the
> maintainer knows that is good to apply differently from the list if
> needed).

I think you need to re-read what those tags mean.

  Documentation/SubmittingPatches

> Now, that doesn't mean that I provide a pull request for the drivers
> or boards I maintain on every release since that will depend on the
> number of patches posted for that component per release. So if there
> are only a couple of patches, I think is easier for the subsystem
> maintainer to pick those directly from the list but if there are a lot
> of them, then the maintainer may ask me to prepare a branch to pull
> and I've done in the past for drivers I maintain to be sure that the
> patches in the list are applied in the right order, no needed patches
> were missed, etc.

I have also submitted patches via a pull-request as a Submitter to
different subsystems.  That does not mean I should automatically be
classed as a Maintainer.

> Another difference is that when I'm listed as a maintainer, I feel an
> obligation to answer to the patches touching that component but that's
> not the case for components I usually act as a reviewer, I may review
> it if I have time but if I don't, I let other people to review it.

Then, in the latter case you shouldn't be listed as a Reviewer in my
example.  Anyone listed as a Reviewer in MAINTAINERS *does* have that
obligation.  That's what it means.  If Reviewers don't review, they
should be removed from MAINTAINERS.

> >> At least for Samsung Multifunction PMIC drivers (and some of Maxim
> >> MUICs and PMICs) these are actively used by us in existing and new
> >> products. They are also continuously extended and actually maintained.
> >> This means that it is not only about review of new patches but also
> >> about caring that nothing will become broken.
> >
> > Exactly.  This what I expect of any good code Reviewer.
> >
> >> I would prefer to leave the "SAMSUNG MULTIFUNCTION PMIC DEVICE
> >> DRIVERS" entry as is - maintainers.
> 
> I agree with Krzysztof here, I would prefer to keep them as
> maintainers if they are maintaining the drivers.

But they're not.  They're reviewing and caring like a good Reviewer
should.

> > But you aren't maintaining the driver i.e. you don't collect patches
> > and *maintain* them on an upstream branch.  Granted some of you guys
> > are doing a great job of maintaining branches on your downstream or
> > BSP kernels, but conduct a Reviewer type role for upstream.
> >
> > You guys are pushing back like this is some kind of demotion.  That's
> > not the case at all.  All it does is better describe the (very worthy)
> > function you *actually* provide.
> >
> 
> But I think it makes description less accurate in fact, since without
> $SUBJECT get_maintainers.pl reports for example:
> 
> Krzysztof Kozlowski <k.kozlowski@samsung.com> (supporter:MAXIM PMIC
> AND MUIC DRIVERS FOR EXYNOS BASED BO...)
> Lee Jones <lee.jones@linaro.org> (supporter:MULTIFUNCTION DEVICES (MFD))
> 
> and after the change:
> 
> Krzysztof Kozlowski <k.kozlowski@samsung.com> (reviewer)
> Lee Jones <lee.jones@linaro.org> (supporter:MULTIFUNCTION DEVICES (MFD))
> 
> He also works for Samsung so the driver is not only maintained but
> supported since he can actually take care of it as a part of his day
> job (if I understood correctly).

It's not the person that's supported, it's the driver.  The driver
doesn't need to change state.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Javier Martinez Canillas]

Hello Lee,

On Wed, Oct 28, 2015 at 11:28 AM, Lee Jones <lee.jones@linaro.org> wrote:
> On Wed, 28 Oct 2015, Javier Martinez Canillas wrote:
>> On Wed, Oct 28, 2015 at 9:24 AM, Lee Jones <lee.jones@linaro.org> wrote:
>> > On Tue, 2015-10-27 at 18:15 +0000, Lee Jones wrote:
>> >> On Tue, 27 Oct 2015, Sebastian Reichel wrote:
>> >> > On Tue, Oct 27, 2015 at 03:42:37PM +0000, Lee Jones wrote:
>> >> > > Since eafbaac ("MAINTAINERS: Add "R:" designated-reviewers tag") we
>> >> > > have been able to tag specific people as Reviewers.  These are key
>> >> > > individuals who are tasked with or volunteer to review code submitted
>> >> > > to a subsystem or specific file.  However, according to MAINTAINERS
>> >> > > we have 1046 Maintainers and only a mere 22 Reviewers.  I believe
>> >> > > these numbers to be incorrect, as many of these Maintainers are in
>> >> > > fact Reviewers.
>> >
>> > Most entries in MAINTAINERS seem to be vanity entries than actual
>> > active participants.  A person typically writes a driver, adds a
>> > MAINTAINER entry, then forgets about it and/or the hardware becomes
>> > outdated.
>> >
>> > This I agree with.
>> >
>> > On Wed, 28 Oct 2015, Krzysztof Kozlowski wrote:
>> >> 2015-10-28 3:44 GMT+09:00 Joe Perches <joe@perches.com>:
>> >> > On Tue, 2015-10-27 at 18:15 +0000, Lee Jones wrote:
>> >> > > On Tue, 27 Oct 2015, Sebastian Reichel wrote:> >
>> >> > > > I think you should CC the people, which are changed from "M:" to
>> >> > > > "R:", though.
>> >> > >
>> >> > > Yes, makes sense.
>> >> > >
>> >> > > I'd like to collect some Maintainer Acks first though.
>> >> >
>> >> > I think people from organizations like Samsung are actual
>> >> > maintainers not reviewers.
>> >
>> > So this all hinges on how we are describing Maintainers and Reviewers.
>> >
>> > My personal definition (until convinced otherwise) is that Reviewers
>> > care about their particular subsystem and/or files.  They conduct code
>> > reviews to ensure nothing gets broken and the code base stays in best
>> > possible state of worthiness.  On the other hand Maintainers usually
>> > conduct themselves as Reviewers but also have 'maintainership' duties
>> > as well; such as applying patches, *maintaining*, testing, rebasing,
>> > etc, an upstream branch and ultimately sending pull-requests to higher
>> > level Maintainers i.e. Linus.  Maintainers also have the ultimate say
>> > (unless over-ruled by Linus etc) over what gets applied.
>> >
>> >> > Their drivers are not thrown over a wall and forgotten.
>> >>
>>
>> I've a different definition. For me it depends on much do you care
>> about the component. For example I maintain a couple of drivers in the
>> kernel and Device Tree files for some boards that are important to me
>> but I also care about some other subsystems (i.e: Exynos SoC support)
>> and I act as a reviewer (although I'm not officially listed as
>> reviewer in the MAINTAINERS file).
>
> I wish to make this clear from the out-set.  If you have no obligation
> to review patches but do so occasionally anyway, that does not make
> you the type of Reviewer that we're speaking about here.  Anyone can
> review any patch on the list that they wish to, which is lovely, but
> it won't carry the same authority (for want of a better expression) as
> if you were tagged as an official Reviewer in MAINTAINERS.
>

We agree on that.

>> We do have in fact different tags for each type of involvement so I
>> usually answer with a Reviewed-by tag if I review code for a subsystem
>> I care but I don't maintainer or answer with an Acked-by tag if I
>> review *and agree* with a patch for a component I maintain (so the
>> maintainer knows that is good to apply differently from the list if
>> needed).
>
> I think you need to re-read what those tags mean.
>
>   Documentation/SubmittingPatches
>

I know that document of course but I went and read the tags
description again and I don't see how that document supports your
arguments. Can you please share the paragraphs you are referring to?

>> Now, that doesn't mean that I provide a pull request for the drivers
>> or boards I maintain on every release since that will depend on the
>> number of patches posted for that component per release. So if there
>> are only a couple of patches, I think is easier for the subsystem
>> maintainer to pick those directly from the list but if there are a lot
>> of them, then the maintainer may ask me to prepare a branch to pull
>> and I've done in the past for drivers I maintain to be sure that the
>> patches in the list are applied in the right order, no needed patches
>> were missed, etc.
>
> I have also submitted patches via a pull-request as a Submitter to
> different subsystems.  That does not mean I should automatically be
> classed as a Maintainer.
>

Yes I agree that preparing pull requests doesn't make you a maintainer
but you are the one using maintain a branch / sending pull requests as
classification method. My point is that this is orthogonal to being a
maintainer or reviewer.

>> Another difference is that when I'm listed as a maintainer, I feel an
>> obligation to answer to the patches touching that component but that's
>> not the case for components I usually act as a reviewer, I may review
>> it if I have time but if I don't, I let other people to review it.
>
> Then, in the latter case you shouldn't be listed as a Reviewer in my
> example.  Anyone listed as a Reviewer in MAINTAINERS *does* have that
> obligation.  That's what it means.  If Reviewers don't review, they
> should be removed from MAINTAINERS.
>

Again we agree on that, that's why I said that I'm *not* officially
listed as a reviewer for Exynos SoC patches since even when I'm
interested on that, I don't have time to review every single patch.

>> >> At least for Samsung Multifunction PMIC drivers (and some of Maxim
>> >> MUICs and PMICs) these are actively used by us in existing and new
>> >> products. They are also continuously extended and actually maintained.
>> >> This means that it is not only about review of new patches but also
>> >> about caring that nothing will become broken.
>> >
>> > Exactly.  This what I expect of any good code Reviewer.
>> >
>> >> I would prefer to leave the "SAMSUNG MULTIFUNCTION PMIC DEVICE
>> >> DRIVERS" entry as is - maintainers.
>>
>> I agree with Krzysztof here, I would prefer to keep them as
>> maintainers if they are maintaining the drivers.
>
> But they're not.  They're reviewing and caring like a good Reviewer
> should.
>

That's your opinion, as I said my opinion is that they are maintaining
it because they care that the drivers are in good shape, testing that
no regressions are introduced, fixing bugs, etc.

>> > But you aren't maintaining the driver i.e. you don't collect patches
>> > and *maintain* them on an upstream branch.  Granted some of you guys
>> > are doing a great job of maintaining branches on your downstream or
>> > BSP kernels, but conduct a Reviewer type role for upstream.
>> >
>> > You guys are pushing back like this is some kind of demotion.  That's
>> > not the case at all.  All it does is better describe the (very worthy)
>> > function you *actually* provide.
>> >
>>
>> But I think it makes description less accurate in fact, since without
>> $SUBJECT get_maintainers.pl reports for example:
>>
>> Krzysztof Kozlowski <k.kozlowski@samsung.com> (supporter:MAXIM PMIC
>> AND MUIC DRIVERS FOR EXYNOS BASED BO...)
>> Lee Jones <lee.jones@linaro.org> (supporter:MULTIFUNCTION DEVICES (MFD))
>>
>> and after the change:
>>
>> Krzysztof Kozlowski <k.kozlowski@samsung.com> (reviewer)
>> Lee Jones <lee.jones@linaro.org> (supporter:MULTIFUNCTION DEVICES (MFD))
>>
>> He also works for Samsung so the driver is not only maintained but
>> supported since he can actually take care of it as a part of his day
>> job (if I understood correctly).
>
> It's not the person that's supported, it's the driver.  The driver
> doesn't need to change state.
>

Yes, is the driver that is supported but by whom? In my opinion the
supporter should be the maintainer of the driver and that is what
get_maintainer.pl thinks as well.

So in summary, you think that the difference between a maintainer and
a reviewer is if a branch with fixes / new features are kept and pull
requests sent while I think that the difference is the level of
involvement someone has with a driver regardless of how patches ends
in the subsystem tree (picked directly by subsystem maintainers or
sent through pull requests).

Is the first time I heard your definition but maybe I'm the one that
is wrong so it would be great to get a consensus on that and get it
documented somewhere.

> --
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog

Best regards,
Javier

[PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Javier Martinez Canillas]

Hello Lee,

On Wed, Oct 28, 2015 at 11:28 AM, Lee Jones <lee.jones@linaro.org> wrote:
> On Wed, 28 Oct 2015, Javier Martinez Canillas wrote:
>> On Wed, Oct 28, 2015 at 9:24 AM, Lee Jones <lee.jones@linaro.org> wrote:
>> > On Tue, 2015-10-27 at 18:15 +0000, Lee Jones wrote:
>> >> On Tue, 27 Oct 2015, Sebastian Reichel wrote:
>> >> > On Tue, Oct 27, 2015 at 03:42:37PM +0000, Lee Jones wrote:
>> >> > > Since eafbaac ("MAINTAINERS: Add "R:" designated-reviewers tag") we
>> >> > > have been able to tag specific people as Reviewers.  These are key
>> >> > > individuals who are tasked with or volunteer to review code submitted
>> >> > > to a subsystem or specific file.  However, according to MAINTAINERS
>> >> > > we have 1046 Maintainers and only a mere 22 Reviewers.  I believe
>> >> > > these numbers to be incorrect, as many of these Maintainers are in
>> >> > > fact Reviewers.
>> >
>> > Most entries in MAINTAINERS seem to be vanity entries than actual
>> > active participants.  A person typically writes a driver, adds a
>> > MAINTAINER entry, then forgets about it and/or the hardware becomes
>> > outdated.
>> >
>> > This I agree with.
>> >
>> > On Wed, 28 Oct 2015, Krzysztof Kozlowski wrote:
>> >> 2015-10-28 3:44 GMT+09:00 Joe Perches <joe@perches.com>:
>> >> > On Tue, 2015-10-27 at 18:15 +0000, Lee Jones wrote:
>> >> > > On Tue, 27 Oct 2015, Sebastian Reichel wrote:> >
>> >> > > > I think you should CC the people, which are changed from "M:" to
>> >> > > > "R:", though.
>> >> > >
>> >> > > Yes, makes sense.
>> >> > >
>> >> > > I'd like to collect some Maintainer Acks first though.
>> >> >
>> >> > I think people from organizations like Samsung are actual
>> >> > maintainers not reviewers.
>> >
>> > So this all hinges on how we are describing Maintainers and Reviewers.
>> >
>> > My personal definition (until convinced otherwise) is that Reviewers
>> > care about their particular subsystem and/or files.  They conduct code
>> > reviews to ensure nothing gets broken and the code base stays in best
>> > possible state of worthiness.  On the other hand Maintainers usually
>> > conduct themselves as Reviewers but also have 'maintainership' duties
>> > as well; such as applying patches, *maintaining*, testing, rebasing,
>> > etc, an upstream branch and ultimately sending pull-requests to higher
>> > level Maintainers i.e. Linus.  Maintainers also have the ultimate say
>> > (unless over-ruled by Linus etc) over what gets applied.
>> >
>> >> > Their drivers are not thrown over a wall and forgotten.
>> >>
>>
>> I've a different definition. For me it depends on much do you care
>> about the component. For example I maintain a couple of drivers in the
>> kernel and Device Tree files for some boards that are important to me
>> but I also care about some other subsystems (i.e: Exynos SoC support)
>> and I act as a reviewer (although I'm not officially listed as
>> reviewer in the MAINTAINERS file).
>
> I wish to make this clear from the out-set.  If you have no obligation
> to review patches but do so occasionally anyway, that does not make
> you the type of Reviewer that we're speaking about here.  Anyone can
> review any patch on the list that they wish to, which is lovely, but
> it won't carry the same authority (for want of a better expression) as
> if you were tagged as an official Reviewer in MAINTAINERS.
>

We agree on that.

>> We do have in fact different tags for each type of involvement so I
>> usually answer with a Reviewed-by tag if I review code for a subsystem
>> I care but I don't maintainer or answer with an Acked-by tag if I
>> review *and agree* with a patch for a component I maintain (so the
>> maintainer knows that is good to apply differently from the list if
>> needed).
>
> I think you need to re-read what those tags mean.
>
>   Documentation/SubmittingPatches
>

I know that document of course but I went and read the tags
description again and I don't see how that document supports your
arguments. Can you please share the paragraphs you are referring to?

>> Now, that doesn't mean that I provide a pull request for the drivers
>> or boards I maintain on every release since that will depend on the
>> number of patches posted for that component per release. So if there
>> are only a couple of patches, I think is easier for the subsystem
>> maintainer to pick those directly from the list but if there are a lot
>> of them, then the maintainer may ask me to prepare a branch to pull
>> and I've done in the past for drivers I maintain to be sure that the
>> patches in the list are applied in the right order, no needed patches
>> were missed, etc.
>
> I have also submitted patches via a pull-request as a Submitter to
> different subsystems.  That does not mean I should automatically be
> classed as a Maintainer.
>

Yes I agree that preparing pull requests doesn't make you a maintainer
but you are the one using maintain a branch / sending pull requests as
classification method. My point is that this is orthogonal to being a
maintainer or reviewer.

>> Another difference is that when I'm listed as a maintainer, I feel an
>> obligation to answer to the patches touching that component but that's
>> not the case for components I usually act as a reviewer, I may review
>> it if I have time but if I don't, I let other people to review it.
>
> Then, in the latter case you shouldn't be listed as a Reviewer in my
> example.  Anyone listed as a Reviewer in MAINTAINERS *does* have that
> obligation.  That's what it means.  If Reviewers don't review, they
> should be removed from MAINTAINERS.
>

Again we agree on that, that's why I said that I'm *not* officially
listed as a reviewer for Exynos SoC patches since even when I'm
interested on that, I don't have time to review every single patch.

>> >> At least for Samsung Multifunction PMIC drivers (and some of Maxim
>> >> MUICs and PMICs) these are actively used by us in existing and new
>> >> products. They are also continuously extended and actually maintained.
>> >> This means that it is not only about review of new patches but also
>> >> about caring that nothing will become broken.
>> >
>> > Exactly.  This what I expect of any good code Reviewer.
>> >
>> >> I would prefer to leave the "SAMSUNG MULTIFUNCTION PMIC DEVICE
>> >> DRIVERS" entry as is - maintainers.
>>
>> I agree with Krzysztof here, I would prefer to keep them as
>> maintainers if they are maintaining the drivers.
>
> But they're not.  They're reviewing and caring like a good Reviewer
> should.
>

That's your opinion, as I said my opinion is that they are maintaining
it because they care that the drivers are in good shape, testing that
no regressions are introduced, fixing bugs, etc.

>> > But you aren't maintaining the driver i.e. you don't collect patches
>> > and *maintain* them on an upstream branch.  Granted some of you guys
>> > are doing a great job of maintaining branches on your downstream or
>> > BSP kernels, but conduct a Reviewer type role for upstream.
>> >
>> > You guys are pushing back like this is some kind of demotion.  That's
>> > not the case at all.  All it does is better describe the (very worthy)
>> > function you *actually* provide.
>> >
>>
>> But I think it makes description less accurate in fact, since without
>> $SUBJECT get_maintainers.pl reports for example:
>>
>> Krzysztof Kozlowski <k.kozlowski@samsung.com> (supporter:MAXIM PMIC
>> AND MUIC DRIVERS FOR EXYNOS BASED BO...)
>> Lee Jones <lee.jones@linaro.org> (supporter:MULTIFUNCTION DEVICES (MFD))
>>
>> and after the change:
>>
>> Krzysztof Kozlowski <k.kozlowski@samsung.com> (reviewer)
>> Lee Jones <lee.jones@linaro.org> (supporter:MULTIFUNCTION DEVICES (MFD))
>>
>> He also works for Samsung so the driver is not only maintained but
>> supported since he can actually take care of it as a part of his day
>> job (if I understood correctly).
>
> It's not the person that's supported, it's the driver.  The driver
> doesn't need to change state.
>

Yes, is the driver that is supported but by whom? In my opinion the
supporter should be the maintainer of the driver and that is what
get_maintainer.pl thinks as well.

So in summary, you think that the difference between a maintainer and
a reviewer is if a branch with fixes / new features are kept and pull
requests sent while I think that the difference is the level of
involvement someone has with a driver regardless of how patches ends
in the subsystem tree (picked directly by subsystem maintainers or
sent through pull requests).

Is the first time I heard your definition but maybe I'm the one that
is wrong so it would be great to get a consensus on that and get it
documented somewhere.

> --
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead
> Linaro.org ? Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog

Best regards,
Javier

Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Joe Perches]

On Wed, 2015-10-28 at 11:53 +0100, Javier Martinez Canillas wrote:
> (Lee) think(s) that the difference between a maintainer and
> a reviewer is if a branch with fixes / new features are kept and pull
> requests sent while I think that the difference is the level of
> involvement someone has with a driver regardless of how patches ends
> in the subsystem tree (picked directly by subsystem maintainers or
> sent through pull requests).
> 
> Is the first time I heard your definition but maybe I'm the one that
> is wrong so it would be great to get a consensus on that and get it
> documented somewhere.

I think Lee is over-analyzing.

>From MAINTAINERS:
	M: Mail patches to: FullName <address@domain>
	R: Designated reviewer: FullName <address@domain>
	   These reviewers should be CCed on patches.
	S: Status, one of the following:
	   Supported:	Someone is actually paid to look after this.
	   Maintained:	Someone actually looks after it.

"looking after" doesn't mean upstreaming.

The original threads for this were:

http://lists.linuxfoundation.org/pipermail/ksummit-discuss/2014-May/000830.html
https://lkml.org/lkml/2014/6/2/446

[PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Joe Perches]

On Wed, 2015-10-28 at 11:53 +0100, Javier Martinez Canillas wrote:
> (Lee) think(s) that the difference between a maintainer and
> a reviewer is if a branch with fixes / new features are kept and pull
> requests sent while I think that the difference is the level of
> involvement someone has with a driver regardless of how patches ends
> in the subsystem tree (picked directly by subsystem maintainers or
> sent through pull requests).
> 
> Is the first time I heard your definition but maybe I'm the one that
> is wrong so it would be great to get a consensus on that and get it
> documented somewhere.

I think Lee is over-analyzing.

>From MAINTAINERS:
	M: Mail patches to: FullName <address@domain>
	R: Designated reviewer: FullName <address@domain>
	   These reviewers should be CCed on patches.
	S: Status, one of the following:
	   Supported:	Someone is actually paid to look after this.
	   Maintained:	Someone actually looks after it.

"looking after" doesn't mean upstreaming.

The original threads for this were:

http://lists.linuxfoundation.org/pipermail/ksummit-discuss/2014-May/000830.html
https://lkml.org/lkml/2014/6/2/446

Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Javier Martinez Canillas]

Hello Joe,

On Wed, Oct 28, 2015 at 12:06 PM, Joe Perches <joe@perches.com> wrote:
> On Wed, 2015-10-28 at 11:53 +0100, Javier Martinez Canillas wrote:
>> (Lee) think(s) that the difference between a maintainer and
>> a reviewer is if a branch with fixes / new features are kept and pull
>> requests sent while I think that the difference is the level of
>> involvement someone has with a driver regardless of how patches ends
>> in the subsystem tree (picked directly by subsystem maintainers or
>> sent through pull requests).
>>
>> Is the first time I heard your definition but maybe I'm the one that
>> is wrong so it would be great to get a consensus on that and get it
>> documented somewhere.
>
> I think Lee is over-analyzing.
>
> From MAINTAINERS:
>         M: Mail patches to: FullName <address@domain>
>         R: Designated reviewer: FullName <address@domain>
>            These reviewers should be CCed on patches.
>         S: Status, one of the following:
>            Supported:   Someone is actually paid to look after this.
>            Maintained:  Someone actually looks after it.
>
> "looking after" doesn't mean upstreaming.
>

Agreed and upstreaming doesn't mean sending pull request, you can for
example upstream the downstream changes for a driver you maintain by
posting patches or ack patches others post and let the subsystem
maintainer to pick those (even if you are listed as the driver
maintainer in MAINTAINERS).

So by following Lee's definition, then most drivers' maintainers
should not be called maintainers since keeping a tree with patches for
both fixes and new features, sending pull requests, etc is only
justified for drivers that have a lot of changes per release. Is not
worth it for drivers that are in "maintenance mode" where only bugs
are fixed every once in a while or features are seldom added.

> The original threads for this were:
>
> http://lists.linuxfoundation.org/pipermail/ksummit-discuss/2014-May/000830.html
> https://lkml.org/lkml/2014/6/2/446
>
>

Thanks for the pointer.

Best regards,
Javier

[PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Javier Martinez Canillas]

Hello Joe,

On Wed, Oct 28, 2015 at 12:06 PM, Joe Perches <joe@perches.com> wrote:
> On Wed, 2015-10-28 at 11:53 +0100, Javier Martinez Canillas wrote:
>> (Lee) think(s) that the difference between a maintainer and
>> a reviewer is if a branch with fixes / new features are kept and pull
>> requests sent while I think that the difference is the level of
>> involvement someone has with a driver regardless of how patches ends
>> in the subsystem tree (picked directly by subsystem maintainers or
>> sent through pull requests).
>>
>> Is the first time I heard your definition but maybe I'm the one that
>> is wrong so it would be great to get a consensus on that and get it
>> documented somewhere.
>
> I think Lee is over-analyzing.
>
> From MAINTAINERS:
>         M: Mail patches to: FullName <address@domain>
>         R: Designated reviewer: FullName <address@domain>
>            These reviewers should be CCed on patches.
>         S: Status, one of the following:
>            Supported:   Someone is actually paid to look after this.
>            Maintained:  Someone actually looks after it.
>
> "looking after" doesn't mean upstreaming.
>

Agreed and upstreaming doesn't mean sending pull request, you can for
example upstream the downstream changes for a driver you maintain by
posting patches or ack patches others post and let the subsystem
maintainer to pick those (even if you are listed as the driver
maintainer in MAINTAINERS).

So by following Lee's definition, then most drivers' maintainers
should not be called maintainers since keeping a tree with patches for
both fixes and new features, sending pull requests, etc is only
justified for drivers that have a lot of changes per release. Is not
worth it for drivers that are in "maintenance mode" where only bugs
are fixed every once in a while or features are seldom added.

> The original threads for this were:
>
> http://lists.linuxfoundation.org/pipermail/ksummit-discuss/2014-May/000830.html
> https://lkml.org/lkml/2014/6/2/446
>
>

Thanks for the pointer.

Best regards,
Javier

Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Lee Jones]

On Wed, 28 Oct 2015, Javier Martinez Canillas wrote:

> Hello Joe,
> 
> On Wed, Oct 28, 2015 at 12:06 PM, Joe Perches <joe@perches.com> wrote:
> > On Wed, 2015-10-28 at 11:53 +0100, Javier Martinez Canillas wrote:
> >> (Lee) think(s) that the difference between a maintainer and
> >> a reviewer is if a branch with fixes / new features are kept and pull
> >> requests sent while I think that the difference is the level of
> >> involvement someone has with a driver regardless of how patches ends
> >> in the subsystem tree (picked directly by subsystem maintainers or
> >> sent through pull requests).
> >>
> >> Is the first time I heard your definition but maybe I'm the one that
> >> is wrong so it would be great to get a consensus on that and get it
> >> documented somewhere.
> >
> > I think Lee is over-analyzing.
> >
> > From MAINTAINERS:
> >         M: Mail patches to: FullName <address@domain>
> >         R: Designated reviewer: FullName <address@domain>
> >            These reviewers should be CCed on patches.
> >         S: Status, one of the following:
> >            Supported:   Someone is actually paid to look after this.
> >            Maintained:  Someone actually looks after it.
> >
> > "looking after" doesn't mean upstreaming.
> >
> 
> Agreed and upstreaming doesn't mean sending pull request, you can for
> example upstream the downstream changes for a driver you maintain by
> posting patches or ack patches others post and let the subsystem
> maintainer to pick those (even if you are listed as the driver
> maintainer in MAINTAINERS).
> 
> So by following Lee's definition, then most drivers' maintainers
> should not be called maintainers since keeping a tree with patches for
> both fixes and new features, sending pull requests, etc is only
> justified for drivers that have a lot of changes per release. Is not
> worth it for drivers that are in "maintenance mode" where only bugs
> are fixed every once in a while or features are seldom added.

Exactly right.

Although, it looks like M: doesn't even mean Maintainer.  If it did, I
would have made these points over and over until death (or until I got
bored).  However, as M: actually means "Mail patches to", there seems
to be very little difference between that and "Designated reviewer"
and makes me wonder why the R: tag was ever even introduced.  I guess
all of the other guys in the threads below also thought M: meant
Maintainer, or else they would have just added poor old Josh as a
"Mail patches to" recipient and been done with it.

> > The original threads for this were:
> >
> > http://lists.linuxfoundation.org/pipermail/ksummit-discuss/2014-May/000830.html
> > https://lkml.org/lkml/2014/6/2/446

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Lee Jones]

On Wed, 28 Oct 2015, Javier Martinez Canillas wrote:

> Hello Joe,
> 
> On Wed, Oct 28, 2015 at 12:06 PM, Joe Perches <joe@perches.com> wrote:
> > On Wed, 2015-10-28 at 11:53 +0100, Javier Martinez Canillas wrote:
> >> (Lee) think(s) that the difference between a maintainer and
> >> a reviewer is if a branch with fixes / new features are kept and pull
> >> requests sent while I think that the difference is the level of
> >> involvement someone has with a driver regardless of how patches ends
> >> in the subsystem tree (picked directly by subsystem maintainers or
> >> sent through pull requests).
> >>
> >> Is the first time I heard your definition but maybe I'm the one that
> >> is wrong so it would be great to get a consensus on that and get it
> >> documented somewhere.
> >
> > I think Lee is over-analyzing.
> >
> > From MAINTAINERS:
> >         M: Mail patches to: FullName <address@domain>
> >         R: Designated reviewer: FullName <address@domain>
> >            These reviewers should be CCed on patches.
> >         S: Status, one of the following:
> >            Supported:   Someone is actually paid to look after this.
> >            Maintained:  Someone actually looks after it.
> >
> > "looking after" doesn't mean upstreaming.
> >
> 
> Agreed and upstreaming doesn't mean sending pull request, you can for
> example upstream the downstream changes for a driver you maintain by
> posting patches or ack patches others post and let the subsystem
> maintainer to pick those (even if you are listed as the driver
> maintainer in MAINTAINERS).
> 
> So by following Lee's definition, then most drivers' maintainers
> should not be called maintainers since keeping a tree with patches for
> both fixes and new features, sending pull requests, etc is only
> justified for drivers that have a lot of changes per release. Is not
> worth it for drivers that are in "maintenance mode" where only bugs
> are fixed every once in a while or features are seldom added.

Exactly right.

Although, it looks like M: doesn't even mean Maintainer.  If it did, I
would have made these points over and over until death (or until I got
bored).  However, as M: actually means "Mail patches to", there seems
to be very little difference between that and "Designated reviewer"
and makes me wonder why the R: tag was ever even introduced.  I guess
all of the other guys in the threads below also thought M: meant
Maintainer, or else they would have just added poor old Josh as a
"Mail patches to" recipient and been done with it.

> > The original threads for this were:
> >
> > http://lists.linuxfoundation.org/pipermail/ksummit-discuss/2014-May/000830.html
> > https://lkml.org/lkml/2014/6/2/446

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Lee Jones]

On Wed, 28 Oct 2015, Lee Jones wrote:

> On Wed, 28 Oct 2015, Javier Martinez Canillas wrote:
> 
> > Hello Joe,
> > 
> > On Wed, Oct 28, 2015 at 12:06 PM, Joe Perches <joe@perches.com> wrote:
> > > On Wed, 2015-10-28 at 11:53 +0100, Javier Martinez Canillas wrote:
> > >> (Lee) think(s) that the difference between a maintainer and
> > >> a reviewer is if a branch with fixes / new features are kept and pull
> > >> requests sent while I think that the difference is the level of
> > >> involvement someone has with a driver regardless of how patches ends
> > >> in the subsystem tree (picked directly by subsystem maintainers or
> > >> sent through pull requests).
> > >>
> > >> Is the first time I heard your definition but maybe I'm the one that
> > >> is wrong so it would be great to get a consensus on that and get it
> > >> documented somewhere.
> > >
> > > I think Lee is over-analyzing.
> > >
> > > From MAINTAINERS:
> > >         M: Mail patches to: FullName <address@domain>
> > >         R: Designated reviewer: FullName <address@domain>
> > >            These reviewers should be CCed on patches.
> > >         S: Status, one of the following:
> > >            Supported:   Someone is actually paid to look after this.
> > >            Maintained:  Someone actually looks after it.
> > >
> > > "looking after" doesn't mean upstreaming.
> > >
> > 
> > Agreed and upstreaming doesn't mean sending pull request, you can for
> > example upstream the downstream changes for a driver you maintain by
> > posting patches or ack patches others post and let the subsystem
> > maintainer to pick those (even if you are listed as the driver
> > maintainer in MAINTAINERS).
> > 
> > So by following Lee's definition, then most drivers' maintainers
> > should not be called maintainers since keeping a tree with patches for
> > both fixes and new features, sending pull requests, etc is only
> > justified for drivers that have a lot of changes per release. Is not
> > worth it for drivers that are in "maintenance mode" where only bugs
> > are fixed every once in a while or features are seldom added.
> 
> Exactly right.
> 
> Although, it looks like M: doesn't even mean Maintainer.  If it did, I
> would have made these points over and over until death (or until I got
> bored).  However, as M: actually means "Mail patches to", there seems
> to be very little difference between that and "Designated reviewer"
> and makes me wonder why the R: tag was ever even introduced.  I guess
> all of the other guys in the threads below also thought M: meant
> Maintainer, or else they would have just added poor old Josh as a
> "Mail patches to" recipient and been done with it.

Ah, but wait.  get_maintainer.pl *does* assume M means Maintainer
doesn't it?  Which is why this came about.  So if we have a "Mail
patches to" entry, get_maintainer.pl tells the user that this is a
Maintainer, which (given that there are over 1000 unique M: entries
and I know that there are no where near that many actual Maintainers)
means that it's printing out incorrect information most of the time.

So back to my original thought then, what can we do to rectify this
situation and make the information printed more meaningful.  Again,
I'm back to using the R: tag appropriately.

Any (technical, which aren't based on "but I really want to be listed
as a Maintainer") thoughts?

> > > The original threads for this were:
> > >
> > > http://lists.linuxfoundation.org/pipermail/ksummit-discuss/2014-May/000830.html
> > > https://lkml.org/lkml/2014/6/2/446

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Lee Jones]

On Wed, 28 Oct 2015, Lee Jones wrote:

> On Wed, 28 Oct 2015, Javier Martinez Canillas wrote:
> 
> > Hello Joe,
> > 
> > On Wed, Oct 28, 2015 at 12:06 PM, Joe Perches <joe@perches.com> wrote:
> > > On Wed, 2015-10-28 at 11:53 +0100, Javier Martinez Canillas wrote:
> > >> (Lee) think(s) that the difference between a maintainer and
> > >> a reviewer is if a branch with fixes / new features are kept and pull
> > >> requests sent while I think that the difference is the level of
> > >> involvement someone has with a driver regardless of how patches ends
> > >> in the subsystem tree (picked directly by subsystem maintainers or
> > >> sent through pull requests).
> > >>
> > >> Is the first time I heard your definition but maybe I'm the one that
> > >> is wrong so it would be great to get a consensus on that and get it
> > >> documented somewhere.
> > >
> > > I think Lee is over-analyzing.
> > >
> > > From MAINTAINERS:
> > >         M: Mail patches to: FullName <address@domain>
> > >         R: Designated reviewer: FullName <address@domain>
> > >            These reviewers should be CCed on patches.
> > >         S: Status, one of the following:
> > >            Supported:   Someone is actually paid to look after this.
> > >            Maintained:  Someone actually looks after it.
> > >
> > > "looking after" doesn't mean upstreaming.
> > >
> > 
> > Agreed and upstreaming doesn't mean sending pull request, you can for
> > example upstream the downstream changes for a driver you maintain by
> > posting patches or ack patches others post and let the subsystem
> > maintainer to pick those (even if you are listed as the driver
> > maintainer in MAINTAINERS).
> > 
> > So by following Lee's definition, then most drivers' maintainers
> > should not be called maintainers since keeping a tree with patches for
> > both fixes and new features, sending pull requests, etc is only
> > justified for drivers that have a lot of changes per release. Is not
> > worth it for drivers that are in "maintenance mode" where only bugs
> > are fixed every once in a while or features are seldom added.
> 
> Exactly right.
> 
> Although, it looks like M: doesn't even mean Maintainer.  If it did, I
> would have made these points over and over until death (or until I got
> bored).  However, as M: actually means "Mail patches to", there seems
> to be very little difference between that and "Designated reviewer"
> and makes me wonder why the R: tag was ever even introduced.  I guess
> all of the other guys in the threads below also thought M: meant
> Maintainer, or else they would have just added poor old Josh as a
> "Mail patches to" recipient and been done with it.

Ah, but wait.  get_maintainer.pl *does* assume M means Maintainer
doesn't it?  Which is why this came about.  So if we have a "Mail
patches to" entry, get_maintainer.pl tells the user that this is a
Maintainer, which (given that there are over 1000 unique M: entries
and I know that there are no where near that many actual Maintainers)
means that it's printing out incorrect information most of the time.

So back to my original thought then, what can we do to rectify this
situation and make the information printed more meaningful.  Again,
I'm back to using the R: tag appropriately.

Any (technical, which aren't based on "but I really want to be listed
as a Maintainer") thoughts?

> > > The original threads for this were:
> > >
> > > http://lists.linuxfoundation.org/pipermail/ksummit-discuss/2014-May/000830.html
> > > https://lkml.org/lkml/2014/6/2/446

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Joe Perches]

On Wed, 2015-10-28 at 12:14 +0000, Lee Jones wrote:
> Ah, but wait.  get_maintainer.pl *does* assume M means Maintainer
> doesn't it?

No, it looks at the "S:" line.

[PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Joe Perches]

On Wed, 2015-10-28 at 12:14 +0000, Lee Jones wrote:
> Ah, but wait.  get_maintainer.pl *does* assume M means Maintainer
> doesn't it?

No, it looks at the "S:" line.

Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Lee Jones]

On Wed, 28 Oct 2015, Joe Perches wrote:
> On Wed, 2015-10-28 at 12:14 +0000, Lee Jones wrote:
> > Ah, but wait.  get_maintainer.pl *does* assume M means Maintainer
> > doesn't it?
> 
> No, it looks at the "S:" line.

Right.  Then assumes because the driver is 'supported' or 'maintained'
that the person(s) listed in M: must be the Supporter(s) or the
Maintainer(s).

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Lee Jones]

On Wed, 28 Oct 2015, Joe Perches wrote:
> On Wed, 2015-10-28 at 12:14 +0000, Lee Jones wrote:
> > Ah, but wait.  get_maintainer.pl *does* assume M means Maintainer
> > doesn't it?
> 
> No, it looks at the "S:" line.

Right.  Then assumes because the driver is 'supported' or 'maintained'
that the person(s) listed in M: must be the Supporter(s) or the
Maintainer(s).

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Joe Perches]

On Wed, 2015-10-28 at 12:24 +0000, Lee Jones wrote:
> On Wed, 28 Oct 2015, Joe Perches wrote:
> > On Wed, 2015-10-28 at 12:14 +0000, Lee Jones wrote:
> > > Ah, but wait.  get_maintainer.pl *does* assume M means Maintainer
> > > doesn't it?
> > 
> > No, it looks at the "S:" line.
> 
> Right.  Then assumes because the driver is 'supported' or 'maintained'
> that the person(s) listed in M: must be the Supporter(s) or the
> Maintainer(s).

Yup, except "assumes" isn't correct.

It's your definition of maintainer that seems to be at odds
with what's otherwise apparently commonly accepted.

Any "M:" entry in a section where the "S:" line is maintained
or supported is generally classified as a maintainer too.

For instance: I think most accept that I am a maintainer of
get_maintainer.pl.

I wrote most of get_maintainer and I accept most but not all
patches to it by acking some and nacking or otherwise requesting
changes in others.  I do not upstream it.

I don't have a git tree at kernel.org and don't really need one.
I rarely send pull requests.  I generally upstream through
Andrew Morton and he uses quilt.  It's working well enough.

The kernel summit thread from last year that initiated the "R:"
line in MAINTAINERS was primarily focused on encouraging new
patch review and honoring those that already take time to review.

http://lists.linuxfoundation.org/pipermail/ksummit-discuss/2014-May/000764.html

Knock your self out about clarifying how process should work.
Generate consensus where necessary but don't try too hard.

It's working reasonably well right now.  Most people are able
to maintain sanity by ignoring what's unimportant to them.

[PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Joe Perches]

On Wed, 2015-10-28 at 12:24 +0000, Lee Jones wrote:
> On Wed, 28 Oct 2015, Joe Perches wrote:
> > On Wed, 2015-10-28 at 12:14 +0000, Lee Jones wrote:
> > > Ah, but wait.  get_maintainer.pl *does* assume M means Maintainer
> > > doesn't it?
> > 
> > No, it looks at the "S:" line.
> 
> Right.  Then assumes because the driver is 'supported' or 'maintained'
> that the person(s) listed in M: must be the Supporter(s) or the
> Maintainer(s).

Yup, except "assumes" isn't correct.

It's your definition of maintainer that seems to be at odds
with what's otherwise apparently commonly accepted.

Any "M:" entry in a section where the "S:" line is maintained
or supported is generally classified as a maintainer too.

For instance: I think most accept that I am a maintainer of
get_maintainer.pl.

I wrote most of get_maintainer and I accept most but not all
patches to it by acking some and nacking or otherwise requesting
changes in others.  I do not upstream it.

I don't have a git tree at kernel.org and don't really need one.
I rarely send pull requests.  I generally upstream through
Andrew Morton and he uses quilt.  It's working well enough.

The kernel summit thread from last year that initiated the "R:"
line in MAINTAINERS was primarily focused on encouraging new
patch review and honoring those that already take time to review.

http://lists.linuxfoundation.org/pipermail/ksummit-discuss/2014-May/000764.html

Knock your self out about clarifying how process should work.
Generate consensus where necessary but don't try too hard.

It's working reasonably well right now.  Most people are able
to maintain sanity by ignoring what's unimportant to them.

Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Javier Martinez Canillas]

Hello Lee,

On Wed, Oct 28, 2015 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:
> On Wed, 28 Oct 2015, Lee Jones wrote:
>
>> On Wed, 28 Oct 2015, Javier Martinez Canillas wrote:
>>
>> > Hello Joe,
>> >
>> > On Wed, Oct 28, 2015 at 12:06 PM, Joe Perches <joe@perches.com> wrote:
>> > > On Wed, 2015-10-28 at 11:53 +0100, Javier Martinez Canillas wrote:
>> > >> (Lee) think(s) that the difference between a maintainer and
>> > >> a reviewer is if a branch with fixes / new features are kept and pull
>> > >> requests sent while I think that the difference is the level of
>> > >> involvement someone has with a driver regardless of how patches ends
>> > >> in the subsystem tree (picked directly by subsystem maintainers or
>> > >> sent through pull requests).
>> > >>
>> > >> Is the first time I heard your definition but maybe I'm the one that
>> > >> is wrong so it would be great to get a consensus on that and get it
>> > >> documented somewhere.
>> > >
>> > > I think Lee is over-analyzing.
>> > >
>> > > From MAINTAINERS:
>> > >         M: Mail patches to: FullName <address@domain>
>> > >         R: Designated reviewer: FullName <address@domain>
>> > >            These reviewers should be CCed on patches.
>> > >         S: Status, one of the following:
>> > >            Supported:   Someone is actually paid to look after this.
>> > >            Maintained:  Someone actually looks after it.
>> > >
>> > > "looking after" doesn't mean upstreaming.
>> > >
>> >
>> > Agreed and upstreaming doesn't mean sending pull request, you can for
>> > example upstream the downstream changes for a driver you maintain by
>> > posting patches or ack patches others post and let the subsystem
>> > maintainer to pick those (even if you are listed as the driver
>> > maintainer in MAINTAINERS).
>> >
>> > So by following Lee's definition, then most drivers' maintainers
>> > should not be called maintainers since keeping a tree with patches for
>> > both fixes and new features, sending pull requests, etc is only
>> > justified for drivers that have a lot of changes per release. Is not
>> > worth it for drivers that are in "maintenance mode" where only bugs
>> > are fixed every once in a while or features are seldom added.
>>
>> Exactly right.
>>
>> Although, it looks like M: doesn't even mean Maintainer.  If it did, I
>> would have made these points over and over until death (or until I got
>> bored).  However, as M: actually means "Mail patches to", there seems
>> to be very little difference between that and "Designated reviewer"
>> and makes me wonder why the R: tag was ever even introduced.  I guess
>> all of the other guys in the threads below also thought M: meant
>> Maintainer, or else they would have just added poor old Josh as a
>> "Mail patches to" recipient and been done with it.
>
> Ah, but wait.  get_maintainer.pl *does* assume M means Maintainer
> doesn't it?  Which is why this came about.  So if we have a "Mail
> patches to" entry, get_maintainer.pl tells the user that this is a

Joe already answered but I'll elaborate a little bit:

"M:" means "Mail patches to" and "S:" means "Status" so what
get_maintainers.pl is reports the person in "M:" printing the status
of the file(s).

So for example if the file has "S: Supported" then the person listed
in "M:" is shown as "supporter" while if the status is "S:
Maintained", the person is listed as "maintainer".

There isn't a "Reviewed" status to specify files that are looked by
"Designated reviewers", it can be added but I don't see the reason of
it.

> Maintainer, which (given that there are over 1000 unique M: entries
> and I know that there are no where near that many actual Maintainers)
> means that it's printing out incorrect information most of the time.
>

Well, that's correct according to your definition of maintainer
(people with a tree that sends pull requests) but I can believe that
there are 1K unique maintainers for different small components
(without taking into account what components may be obsolete / not
used anymore) even if these maintainers don't send pull request
because the patch load is low and rely on subsystem maintainers to
pick directly from the list with their Acks.

For me the maintainer is that a) cares about the file and makes sure
that things remain working, fix issues, reviews patches from others,
etc b) is someone that actually understand the code in the files. A
subsystem maintainer has the last word of what gets merged into the
subsystem but may not be familiar with code under the subsystem and
relies on the file / driver maintainer to Ack the patch as correct
since that person is the one that knows the code.

> So back to my original thought then, what can we do to rectify this
> situation and make the information printed more meaningful.  Again,
> I'm back to using the R: tag appropriately.
>
> Any (technical, which aren't based on "but I really want to be listed
> as a Maintainer") thoughts?
>

I haven't read a "but I really want to be listed as a maintainer"
thought in this thread.

I think the problem is the definition of what a maintainer in Linux
really means. For you is someone that keeps a tree and sends pull
request (for me that is what I call a subsystem maintainer) while for
others it seems that maintainer means someone who care about a set of
files, knows the code, makes sure that things keep working and Ack
patches to those files when posted.

Best regards,
Javier

[PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Javier Martinez Canillas]

Hello Lee,

On Wed, Oct 28, 2015 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:
> On Wed, 28 Oct 2015, Lee Jones wrote:
>
>> On Wed, 28 Oct 2015, Javier Martinez Canillas wrote:
>>
>> > Hello Joe,
>> >
>> > On Wed, Oct 28, 2015 at 12:06 PM, Joe Perches <joe@perches.com> wrote:
>> > > On Wed, 2015-10-28 at 11:53 +0100, Javier Martinez Canillas wrote:
>> > >> (Lee) think(s) that the difference between a maintainer and
>> > >> a reviewer is if a branch with fixes / new features are kept and pull
>> > >> requests sent while I think that the difference is the level of
>> > >> involvement someone has with a driver regardless of how patches ends
>> > >> in the subsystem tree (picked directly by subsystem maintainers or
>> > >> sent through pull requests).
>> > >>
>> > >> Is the first time I heard your definition but maybe I'm the one that
>> > >> is wrong so it would be great to get a consensus on that and get it
>> > >> documented somewhere.
>> > >
>> > > I think Lee is over-analyzing.
>> > >
>> > > From MAINTAINERS:
>> > >         M: Mail patches to: FullName <address@domain>
>> > >         R: Designated reviewer: FullName <address@domain>
>> > >            These reviewers should be CCed on patches.
>> > >         S: Status, one of the following:
>> > >            Supported:   Someone is actually paid to look after this.
>> > >            Maintained:  Someone actually looks after it.
>> > >
>> > > "looking after" doesn't mean upstreaming.
>> > >
>> >
>> > Agreed and upstreaming doesn't mean sending pull request, you can for
>> > example upstream the downstream changes for a driver you maintain by
>> > posting patches or ack patches others post and let the subsystem
>> > maintainer to pick those (even if you are listed as the driver
>> > maintainer in MAINTAINERS).
>> >
>> > So by following Lee's definition, then most drivers' maintainers
>> > should not be called maintainers since keeping a tree with patches for
>> > both fixes and new features, sending pull requests, etc is only
>> > justified for drivers that have a lot of changes per release. Is not
>> > worth it for drivers that are in "maintenance mode" where only bugs
>> > are fixed every once in a while or features are seldom added.
>>
>> Exactly right.
>>
>> Although, it looks like M: doesn't even mean Maintainer.  If it did, I
>> would have made these points over and over until death (or until I got
>> bored).  However, as M: actually means "Mail patches to", there seems
>> to be very little difference between that and "Designated reviewer"
>> and makes me wonder why the R: tag was ever even introduced.  I guess
>> all of the other guys in the threads below also thought M: meant
>> Maintainer, or else they would have just added poor old Josh as a
>> "Mail patches to" recipient and been done with it.
>
> Ah, but wait.  get_maintainer.pl *does* assume M means Maintainer
> doesn't it?  Which is why this came about.  So if we have a "Mail
> patches to" entry, get_maintainer.pl tells the user that this is a

Joe already answered but I'll elaborate a little bit:

"M:" means "Mail patches to" and "S:" means "Status" so what
get_maintainers.pl is reports the person in "M:" printing the status
of the file(s).

So for example if the file has "S: Supported" then the person listed
in "M:" is shown as "supporter" while if the status is "S:
Maintained", the person is listed as "maintainer".

There isn't a "Reviewed" status to specify files that are looked by
"Designated reviewers", it can be added but I don't see the reason of
it.

> Maintainer, which (given that there are over 1000 unique M: entries
> and I know that there are no where near that many actual Maintainers)
> means that it's printing out incorrect information most of the time.
>

Well, that's correct according to your definition of maintainer
(people with a tree that sends pull requests) but I can believe that
there are 1K unique maintainers for different small components
(without taking into account what components may be obsolete / not
used anymore) even if these maintainers don't send pull request
because the patch load is low and rely on subsystem maintainers to
pick directly from the list with their Acks.

For me the maintainer is that a) cares about the file and makes sure
that things remain working, fix issues, reviews patches from others,
etc b) is someone that actually understand the code in the files. A
subsystem maintainer has the last word of what gets merged into the
subsystem but may not be familiar with code under the subsystem and
relies on the file / driver maintainer to Ack the patch as correct
since that person is the one that knows the code.

> So back to my original thought then, what can we do to rectify this
> situation and make the information printed more meaningful.  Again,
> I'm back to using the R: tag appropriately.
>
> Any (technical, which aren't based on "but I really want to be listed
> as a Maintainer") thoughts?
>

I haven't read a "but I really want to be listed as a maintainer"
thought in this thread.

I think the problem is the definition of what a maintainer in Linux
really means. For you is someone that keeps a tree and sends pull
request (for me that is what I call a subsystem maintainer) while for
others it seems that maintainer means someone who care about a set of
files, knows the code, makes sure that things keep working and Ack
patches to those files when posted.

Best regards,
Javier

Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Krzysztof Kozlowski]

W dniu 28.10.2015 o 18:55, Lee Jones pisze:
> On Wed, 28 Oct 2015, Uwe Kleine-König wrote:
> 
>> Hello,
>>
>> On Wed, Oct 28, 2015 at 06:23:37PM +0900, Krzysztof Kozlowski wrote:
>>> On 28.10.2015 17:24, Lee Jones wrote:
>>>> You guys are pushing back like this is some kind of demotion.
>>>> That's not the case at all.  All it does is better describe the (very
>>>> worthy) function you *actually* provide.
>>>
>>> It is getting into dispute about entire change of yours... which is not
>>> what I want. I agree with your general idea but I was referring only to
>>> that particular case - the Samsung PMICs (and Maxim PMICs/MUICs which
>>> would fall into same category).
>>
>> Not being affected by this change, I wonder what the technical
>> difference is if someone is listed as reviewer instead of maintainer.
>> Does get_maintainer.pl behave differently?
> 
> Reviewers are people who should be Cc'ed on all patches, as are
> Maintainers, so no, fundamentally they are treated the same by a
> Submitter.
> 
> Here is some output for Maintainers/Reviews from get_maintainer.pl.
> 
>  $ ./scripts/get_maintainer.pl -f arch/arm/boot/dts/vf500.dtsi
>  Shawn Guo <shawnguo@kernel.org> (maintainer:ARM/FREESCALE VYBRID ARM ARCHITECTURE)
>  Sascha Hauer <kernel@pengutronix.de> (maintainer:ARM/FREESCALE VYBRID ARM ARCHITECTURE)
>  Stefan Agner <stefan@agner.ch> (reviewer)
>  Russell King <linux@arm.linux.org.uk> (maintainer:ARM PORT)
>  linux-arm-kernel@lists.infradead.org (moderated list:ARM/FREESCALE VYBRID ARM ARCHITECTURE)
>  devicetree@vger.kernel.org (open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS)
>  linux-kernel@vger.kernel.org (open list)
> 
> I'm sure we can make the output even more similar by listing the
> MAINTAINERS tag after "reviewer" too.

And here is the output of drivers/mfd/sec-core.c *before*:
Sangbeom Kim <sbkim73@samsung.com> (supporter:SAMSUNG MULTIFUNCTION PMIC DEVICE DRIVERS)
Krzysztof Kozlowski <k.kozlowski@samsung.com> (supporter:SAMSUNG MULTIFUNCTION PMIC DEVICE DRIVERS)
Lee Jones <lee.jones@linaro.org> (supporter:MULTIFUNCTION DEVICES (MFD))


and after (where I am listed as reviewer):
Sangbeom Kim <sbkim73@samsung.com> (reviewer)
Krzysztof Kozlowski <k.kozlowski@samsung.com> (reviewer)
Lee Jones <lee.jones@linaro.org> (supporter:MULTIFUNCTION DEVICES (MFD))

Maybe it should be "supporter reviewer"?

Best regards,
Krzysztof

[PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Krzysztof Kozlowski]

W dniu 28.10.2015 o 18:55, Lee Jones pisze:
> On Wed, 28 Oct 2015, Uwe Kleine-K?nig wrote:
> 
>> Hello,
>>
>> On Wed, Oct 28, 2015 at 06:23:37PM +0900, Krzysztof Kozlowski wrote:
>>> On 28.10.2015 17:24, Lee Jones wrote:
>>>> You guys are pushing back like this is some kind of demotion.
>>>> That's not the case at all.  All it does is better describe the (very
>>>> worthy) function you *actually* provide.
>>>
>>> It is getting into dispute about entire change of yours... which is not
>>> what I want. I agree with your general idea but I was referring only to
>>> that particular case - the Samsung PMICs (and Maxim PMICs/MUICs which
>>> would fall into same category).
>>
>> Not being affected by this change, I wonder what the technical
>> difference is if someone is listed as reviewer instead of maintainer.
>> Does get_maintainer.pl behave differently?
> 
> Reviewers are people who should be Cc'ed on all patches, as are
> Maintainers, so no, fundamentally they are treated the same by a
> Submitter.
> 
> Here is some output for Maintainers/Reviews from get_maintainer.pl.
> 
>  $ ./scripts/get_maintainer.pl -f arch/arm/boot/dts/vf500.dtsi
>  Shawn Guo <shawnguo@kernel.org> (maintainer:ARM/FREESCALE VYBRID ARM ARCHITECTURE)
>  Sascha Hauer <kernel@pengutronix.de> (maintainer:ARM/FREESCALE VYBRID ARM ARCHITECTURE)
>  Stefan Agner <stefan@agner.ch> (reviewer)
>  Russell King <linux@arm.linux.org.uk> (maintainer:ARM PORT)
>  linux-arm-kernel at lists.infradead.org (moderated list:ARM/FREESCALE VYBRID ARM ARCHITECTURE)
>  devicetree at vger.kernel.org (open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS)
>  linux-kernel at vger.kernel.org (open list)
> 
> I'm sure we can make the output even more similar by listing the
> MAINTAINERS tag after "reviewer" too.

And here is the output of drivers/mfd/sec-core.c *before*:
Sangbeom Kim <sbkim73@samsung.com> (supporter:SAMSUNG MULTIFUNCTION PMIC DEVICE DRIVERS)
Krzysztof Kozlowski <k.kozlowski@samsung.com> (supporter:SAMSUNG MULTIFUNCTION PMIC DEVICE DRIVERS)
Lee Jones <lee.jones@linaro.org> (supporter:MULTIFUNCTION DEVICES (MFD))


and after (where I am listed as reviewer):
Sangbeom Kim <sbkim73@samsung.com> (reviewer)
Krzysztof Kozlowski <k.kozlowski@samsung.com> (reviewer)
Lee Jones <lee.jones@linaro.org> (supporter:MULTIFUNCTION DEVICES (MFD))

Maybe it should be "supporter reviewer"?

Best regards,
Krzysztof

Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Krzysztof Kozlowski]

W dniu 28.10.2015 o 19:14, Lee Jones pisze:
> On Wed, 28 Oct 2015, Krzysztof Kozlowski wrote:
>> On 28.10.2015 17:24, Lee Jones wrote:
>>> On Wed, 28 Oct 2015, Krzysztof Kozlowski wrote:
>>>> 2015-10-28 3:44 GMT+09:00 Joe Perches <joe@perches.com>:
>>>>> On Tue, 2015-10-27 at 18:15 +0000, Lee Jones wrote:
>>>>>> On Tue, 27 Oct 2015, Sebastian Reichel wrote:> >
>>>>>>> I think you should CC the people, which are changed from "M:"
>>>>>>> to "R:", though.
>>>>>>
>>>>>> Yes, makes sense.
>>>>>>
>>>>>> I'd like to collect some Maintainer Acks first though.
>>>>>
>>>>> I think people from organizations like Samsung are actual 
>>>>> maintainers not reviewers.
>>>
>>> So this all hinges on how we are describing Maintainers and
>>> Reviewers.
>>>
>>> My personal definition (until convinced otherwise) is that Reviewers 
>>> care about their particular subsystem and/or files.  They conduct
>>> code reviews to ensure nothing gets broken and the code base stays in
>>> best possible state of worthiness.  On the other hand Maintainers
>>> usually conduct themselves as Reviewers but also have
>>> 'maintainership' duties as well; such as applying patches,
>>> *maintaining*, testing, rebasing, etc, an upstream branch and
>>> ultimately sending pull-requests to higher level Maintainers i.e.
>>> Linus.  Maintainers also have the ultimate say (unless over-ruled by
>>> Linus etc) over what gets applied.
>>
>> Okay, sounds reasonable... so if a person performs reviews plus he does
>> some of the other activities (not all) then who is he?
> 
> LT;DR: If someone doesn't *maintain* an upstream branch, they are not
> an upstream Maintainer.

If I understand your point correctly: The maintainer and supporter
should be mentioned if and only if he maintains an upstream branch?

> 
>> For example reviewing
> 
> Depends on the type of engagement.  Anyone can review any patch
> submitted to anywhere on the code-base.  This does not make them an
> accepted Reviewer.  Here I'm saying that a Reviewer is a competent
> engineer who has made a promise to dedicate time to review incoming
> patches in order to ensure quality.
> 
> To emphasise a tagged Reviewer isn't just someone who reviews code
> every now and again.  It's someone who cares and has a vested interest
> in either a subsystem as a whole, or perhaps individual or a group of
> drivers.  However, this person does not conduct upstream branch
> *maintenance*.
> 
>> testing + fixing bugs + cleaning up (sending
>> patches from time to time)?
> 
> This is a Submitter/Contributor.
> 
> When I said testing before, I meant the branch being maintained, not
> the driver on it's own.  That should be tested by Submitters/
> Contributors or Testers (who get to provide their Tested-by).
> 
>> Would that be sufficient requirement to call him maintainer of a driver?
>> Or maybe all of these requirements must be met (including handling of
>> patches and sending pull reqs)?
> 
> It's only really the handing of patches and the maintenance of an
> upstream branch which differentiates a Reviewer from a Maintainer. 
> 
>>>>> Their drivers are not thrown over a wall and forgotten.
>>>>
>>>> At least for Samsung Multifunction PMIC drivers (and some of Maxim 
>>>> MUICs and PMICs) these are actively used by us in existing and new 
>>>> products. They are also continuously extended and actually
>>>> maintained. This means that it is not only about review of new
>>>> patches but also about caring that nothing will become broken.
>>>
>>> Exactly.  This what I expect of any good code Reviewer.
>>>
>>>> I would prefer to leave the "SAMSUNG MULTIFUNCTION PMIC DEVICE 
>>>> DRIVERS" entry as is - maintainers.
>>>
>>> But you aren't maintaining the driver i.e. you don't collect patches 
>>> and *maintain* them on an upstream branch.
>>
>> Indeed, we don't. However are other non-reviewing activities sufficient?
> 
> The other non-reviewing activities you mentioned are that of the
> Submitter/Contributor/Tester.  They still don't make someone a
> Maintainer.

I think I got your point of view. I don't see it that way, especially
that I pointed the fact of combining these activities.
Submitter/Reviewer/Tester in one person.

> 
>>> Granted some of you guys 
>>> are doing a great job of maintaining branches on your downstream or 
>>> BSP kernels, but conduct a Reviewer type role for upstream.
>>
>> You mentioned also the "ultimate say over what gets applied" - which in
>> this particular case is interesting to us because we have direct
>> interest in these drivers being in a good shape and doing things we
>> expect them to do. Like representing the interest of users.
> 
> Any Reviewers opinion will matter to someone who ultimately applies
> the patches.  For instance, if you were to say to me "this change to
> our MFD doesn't suit us because of X", I almost certainly won't apply
> the patch.
> 
>> Of course one could say that every upstreaming person has such
>> expectations... but some of the upstreamers just send a driver for one
>> device. Or extend driver for one device. In this case this is a family
>> of devices used on all of our Exynos SoC products and we care about all
>> of them.
> 
> And being a nominated Reviewer mentioned in MAINTAINERS, that's
> exactly what I'd expect.  If people fail to mean these requirement
> they should be removed completely.

Both of the points above make sense. The person mentioned as reviewer
should review.

> 
>>> You guys are pushing back like this is some kind of demotion.
>>> That's not the case at all.  All it does is better describe the (very
>>> worthy) function you *actually* provide.
>>
>> It is getting into dispute about entire change of yours... which is not
>> what I want. I agree with your general idea but I was referring only to
>> that particular case - the Samsung PMICs (and Maxim PMICs/MUICs which
>> would fall into same category).
> 
> I hope that I've explained my view adequately above.  My view is also
> carried out over the Samsung drivers.

Yes, you explained your point of view... and we can agree to disagree.
:) In the same time I actually accept the fact that I am not the person
with any kind of knowledge about kernel development process.

I think I also described what I am doing with the Samsung drivers so if
that falls under "Reviewing" then I am entirely fine with it.

Best regards,
Krzysztof

[PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Krzysztof Kozlowski]

W dniu 28.10.2015 o 19:14, Lee Jones pisze:
> On Wed, 28 Oct 2015, Krzysztof Kozlowski wrote:
>> On 28.10.2015 17:24, Lee Jones wrote:
>>> On Wed, 28 Oct 2015, Krzysztof Kozlowski wrote:
>>>> 2015-10-28 3:44 GMT+09:00 Joe Perches <joe@perches.com>:
>>>>> On Tue, 2015-10-27 at 18:15 +0000, Lee Jones wrote:
>>>>>> On Tue, 27 Oct 2015, Sebastian Reichel wrote:> >
>>>>>>> I think you should CC the people, which are changed from "M:"
>>>>>>> to "R:", though.
>>>>>>
>>>>>> Yes, makes sense.
>>>>>>
>>>>>> I'd like to collect some Maintainer Acks first though.
>>>>>
>>>>> I think people from organizations like Samsung are actual 
>>>>> maintainers not reviewers.
>>>
>>> So this all hinges on how we are describing Maintainers and
>>> Reviewers.
>>>
>>> My personal definition (until convinced otherwise) is that Reviewers 
>>> care about their particular subsystem and/or files.  They conduct
>>> code reviews to ensure nothing gets broken and the code base stays in
>>> best possible state of worthiness.  On the other hand Maintainers
>>> usually conduct themselves as Reviewers but also have
>>> 'maintainership' duties as well; such as applying patches,
>>> *maintaining*, testing, rebasing, etc, an upstream branch and
>>> ultimately sending pull-requests to higher level Maintainers i.e.
>>> Linus.  Maintainers also have the ultimate say (unless over-ruled by
>>> Linus etc) over what gets applied.
>>
>> Okay, sounds reasonable... so if a person performs reviews plus he does
>> some of the other activities (not all) then who is he?
> 
> LT;DR: If someone doesn't *maintain* an upstream branch, they are not
> an upstream Maintainer.

If I understand your point correctly: The maintainer and supporter
should be mentioned if and only if he maintains an upstream branch?

> 
>> For example reviewing
> 
> Depends on the type of engagement.  Anyone can review any patch
> submitted to anywhere on the code-base.  This does not make them an
> accepted Reviewer.  Here I'm saying that a Reviewer is a competent
> engineer who has made a promise to dedicate time to review incoming
> patches in order to ensure quality.
> 
> To emphasise a tagged Reviewer isn't just someone who reviews code
> every now and again.  It's someone who cares and has a vested interest
> in either a subsystem as a whole, or perhaps individual or a group of
> drivers.  However, this person does not conduct upstream branch
> *maintenance*.
> 
>> testing + fixing bugs + cleaning up (sending
>> patches from time to time)?
> 
> This is a Submitter/Contributor.
> 
> When I said testing before, I meant the branch being maintained, not
> the driver on it's own.  That should be tested by Submitters/
> Contributors or Testers (who get to provide their Tested-by).
> 
>> Would that be sufficient requirement to call him maintainer of a driver?
>> Or maybe all of these requirements must be met (including handling of
>> patches and sending pull reqs)?
> 
> It's only really the handing of patches and the maintenance of an
> upstream branch which differentiates a Reviewer from a Maintainer. 
> 
>>>>> Their drivers are not thrown over a wall and forgotten.
>>>>
>>>> At least for Samsung Multifunction PMIC drivers (and some of Maxim 
>>>> MUICs and PMICs) these are actively used by us in existing and new 
>>>> products. They are also continuously extended and actually
>>>> maintained. This means that it is not only about review of new
>>>> patches but also about caring that nothing will become broken.
>>>
>>> Exactly.  This what I expect of any good code Reviewer.
>>>
>>>> I would prefer to leave the "SAMSUNG MULTIFUNCTION PMIC DEVICE 
>>>> DRIVERS" entry as is - maintainers.
>>>
>>> But you aren't maintaining the driver i.e. you don't collect patches 
>>> and *maintain* them on an upstream branch.
>>
>> Indeed, we don't. However are other non-reviewing activities sufficient?
> 
> The other non-reviewing activities you mentioned are that of the
> Submitter/Contributor/Tester.  They still don't make someone a
> Maintainer.

I think I got your point of view. I don't see it that way, especially
that I pointed the fact of combining these activities.
Submitter/Reviewer/Tester in one person.

> 
>>> Granted some of you guys 
>>> are doing a great job of maintaining branches on your downstream or 
>>> BSP kernels, but conduct a Reviewer type role for upstream.
>>
>> You mentioned also the "ultimate say over what gets applied" - which in
>> this particular case is interesting to us because we have direct
>> interest in these drivers being in a good shape and doing things we
>> expect them to do. Like representing the interest of users.
> 
> Any Reviewers opinion will matter to someone who ultimately applies
> the patches.  For instance, if you were to say to me "this change to
> our MFD doesn't suit us because of X", I almost certainly won't apply
> the patch.
> 
>> Of course one could say that every upstreaming person has such
>> expectations... but some of the upstreamers just send a driver for one
>> device. Or extend driver for one device. In this case this is a family
>> of devices used on all of our Exynos SoC products and we care about all
>> of them.
> 
> And being a nominated Reviewer mentioned in MAINTAINERS, that's
> exactly what I'd expect.  If people fail to mean these requirement
> they should be removed completely.

Both of the points above make sense. The person mentioned as reviewer
should review.

> 
>>> You guys are pushing back like this is some kind of demotion.
>>> That's not the case at all.  All it does is better describe the (very
>>> worthy) function you *actually* provide.
>>
>> It is getting into dispute about entire change of yours... which is not
>> what I want. I agree with your general idea but I was referring only to
>> that particular case - the Samsung PMICs (and Maxim PMICs/MUICs which
>> would fall into same category).
> 
> I hope that I've explained my view adequately above.  My view is also
> carried out over the Samsung drivers.

Yes, you explained your point of view... and we can agree to disagree.
:) In the same time I actually accept the fact that I am not the person
with any kind of knowledge about kernel development process.

I think I also described what I am doing with the Samsung drivers so if
that falls under "Reviewing" then I am entirely fine with it.

Best regards,
Krzysztof

Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Lee Jones]

On Wed, 28 Oct 2015, Javier Martinez Canillas wrote:
> On Wed, Oct 28, 2015 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:
> > On Wed, 28 Oct 2015, Lee Jones wrote:
> >
> >> On Wed, 28 Oct 2015, Javier Martinez Canillas wrote:
> >>
> >> > Hello Joe,
> >> >
> >> > On Wed, Oct 28, 2015 at 12:06 PM, Joe Perches <joe@perches.com> wrote:
> >> > > On Wed, 2015-10-28 at 11:53 +0100, Javier Martinez Canillas wrote:
> >> > >> (Lee) think(s) that the difference between a maintainer and
> >> > >> a reviewer is if a branch with fixes / new features are kept and pull
> >> > >> requests sent while I think that the difference is the level of
> >> > >> involvement someone has with a driver regardless of how patches ends
> >> > >> in the subsystem tree (picked directly by subsystem maintainers or
> >> > >> sent through pull requests).
> >> > >>
> >> > >> Is the first time I heard your definition but maybe I'm the one that
> >> > >> is wrong so it would be great to get a consensus on that and get it
> >> > >> documented somewhere.
> >> > >
> >> > > I think Lee is over-analyzing.
> >> > >
> >> > > From MAINTAINERS:
> >> > >         M: Mail patches to: FullName <address@domain>
> >> > >         R: Designated reviewer: FullName <address@domain>
> >> > >            These reviewers should be CCed on patches.
> >> > >         S: Status, one of the following:
> >> > >            Supported:   Someone is actually paid to look after this.
> >> > >            Maintained:  Someone actually looks after it.
> >> > >
> >> > > "looking after" doesn't mean upstreaming.
> >> > >
> >> >
> >> > Agreed and upstreaming doesn't mean sending pull request, you can for
> >> > example upstream the downstream changes for a driver you maintain by
> >> > posting patches or ack patches others post and let the subsystem
> >> > maintainer to pick those (even if you are listed as the driver
> >> > maintainer in MAINTAINERS).
> >> >
> >> > So by following Lee's definition, then most drivers' maintainers
> >> > should not be called maintainers since keeping a tree with patches for
> >> > both fixes and new features, sending pull requests, etc is only
> >> > justified for drivers that have a lot of changes per release. Is not
> >> > worth it for drivers that are in "maintenance mode" where only bugs
> >> > are fixed every once in a while or features are seldom added.
> >>
> >> Exactly right.
> >>
> >> Although, it looks like M: doesn't even mean Maintainer.  If it did, I
> >> would have made these points over and over until death (or until I got
> >> bored).  However, as M: actually means "Mail patches to", there seems
> >> to be very little difference between that and "Designated reviewer"
> >> and makes me wonder why the R: tag was ever even introduced.  I guess
> >> all of the other guys in the threads below also thought M: meant
> >> Maintainer, or else they would have just added poor old Josh as a
> >> "Mail patches to" recipient and been done with it.
> >
> > Ah, but wait.  get_maintainer.pl *does* assume M means Maintainer
> > doesn't it?  Which is why this came about.  So if we have a "Mail
> > patches to" entry, get_maintainer.pl tells the user that this is a
> 
> Joe already answered but I'll elaborate a little bit:
> 
> "M:" means "Mail patches to" and "S:" means "Status" so what
> get_maintainers.pl is reports the person in "M:" printing the status
> of the file(s).
> 
> So for example if the file has "S: Supported" then the person listed
> in "M:" is shown as "supporter" while if the status is "S:
> Maintained", the person is listed as "maintainer".
> 
> There isn't a "Reviewed" status to specify files that are looked by
> "Designated reviewers", it can be added but I don't see the reason of
> it.

Not sure why you wrote all of this.  We know *why* get_maintainer.pl
does what it does.  What I'm saying is, that I personally believe this
is the wrong behaviour in what I'm *guessing* is the majority of the
time.

> > Maintainer, which (given that there are over 1000 unique M: entries
> > and I know that there are no where near that many actual Maintainers)
> > means that it's printing out incorrect information most of the time.
> 
> Well, that's correct according to your definition of maintainer
> (people with a tree that sends pull requests) but I can believe that
> there are 1K unique maintainers for different small components
> (without taking into account what components may be obsolete / not
> used anymore) even if these maintainers don't send pull request
> because the patch load is low and rely on subsystem maintainers to
> pick directly from the list with their Acks.

Then they are not Maintainers.  They are Reviewers who rely on
Maintainers.  In your example above it's the Maintainers that are
Maintainers and the people who review the code are Reviewers.  The
clues are in the words. ;)

> For me the maintainer is that a) cares about the file and makes sure
> that things remain working, fix issues, reviews patches from others,
> etc b) is someone that actually understand the code in the files. A
> subsystem maintainer has the last word of what gets merged into the
> subsystem but may not be familiar with code under the subsystem and
> relies on the file / driver maintainer to Ack the patch as correct
> since that person is the one that knows the code.

Then what is a Reviewer?

> > So back to my original thought then, what can we do to rectify this
> > situation and make the information printed more meaningful.  Again,
> > I'm back to using the R: tag appropriately.
> >
> > Any (technical, which aren't based on "but I really want to be listed
> > as a Maintainer") thoughts?
> >
> 
> I haven't read a "but I really want to be listed as a maintainer"
> thought in this thread.

If you had no vested interest in this, you would be able to see the
logic in what I'm saying.  Again I'm *guess*, but I think there is
some emotional reasons for you pushing back so hard.  I could always
be wrong though.

> I think the problem is the definition of what a maintainer in Linux
> really means. For you is someone that keeps a tree and sends pull
> request (for me that is what I call a subsystem maintainer)

Right.  A Level 3 Maintainer sends pull-requests to a Level 2
Maintainer, who sends pull-requests to a Level 1 Maintainer, who sends
pull-requests to Linus.  Maintainers at *all* levels collect patches
and Maintain them before sending on.

reiterate:

Reviewers care about particular driver(s)/domain(s) that they know
well.  They provide solid reviews and possible testing (because they
are likely to have the h/w).  They then provide Acked-by:s so that a
Maintainer can collect the patch. 

> while for
> others it seems that maintainer means someone who care about a set of
> files, knows the code, makes sure that things keep working and Ack
> patches to those files when posted.

        goto reiterate;   ;)

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Lee Jones]

On Wed, 28 Oct 2015, Javier Martinez Canillas wrote:
> On Wed, Oct 28, 2015 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:
> > On Wed, 28 Oct 2015, Lee Jones wrote:
> >
> >> On Wed, 28 Oct 2015, Javier Martinez Canillas wrote:
> >>
> >> > Hello Joe,
> >> >
> >> > On Wed, Oct 28, 2015 at 12:06 PM, Joe Perches <joe@perches.com> wrote:
> >> > > On Wed, 2015-10-28 at 11:53 +0100, Javier Martinez Canillas wrote:
> >> > >> (Lee) think(s) that the difference between a maintainer and
> >> > >> a reviewer is if a branch with fixes / new features are kept and pull
> >> > >> requests sent while I think that the difference is the level of
> >> > >> involvement someone has with a driver regardless of how patches ends
> >> > >> in the subsystem tree (picked directly by subsystem maintainers or
> >> > >> sent through pull requests).
> >> > >>
> >> > >> Is the first time I heard your definition but maybe I'm the one that
> >> > >> is wrong so it would be great to get a consensus on that and get it
> >> > >> documented somewhere.
> >> > >
> >> > > I think Lee is over-analyzing.
> >> > >
> >> > > From MAINTAINERS:
> >> > >         M: Mail patches to: FullName <address@domain>
> >> > >         R: Designated reviewer: FullName <address@domain>
> >> > >            These reviewers should be CCed on patches.
> >> > >         S: Status, one of the following:
> >> > >            Supported:   Someone is actually paid to look after this.
> >> > >            Maintained:  Someone actually looks after it.
> >> > >
> >> > > "looking after" doesn't mean upstreaming.
> >> > >
> >> >
> >> > Agreed and upstreaming doesn't mean sending pull request, you can for
> >> > example upstream the downstream changes for a driver you maintain by
> >> > posting patches or ack patches others post and let the subsystem
> >> > maintainer to pick those (even if you are listed as the driver
> >> > maintainer in MAINTAINERS).
> >> >
> >> > So by following Lee's definition, then most drivers' maintainers
> >> > should not be called maintainers since keeping a tree with patches for
> >> > both fixes and new features, sending pull requests, etc is only
> >> > justified for drivers that have a lot of changes per release. Is not
> >> > worth it for drivers that are in "maintenance mode" where only bugs
> >> > are fixed every once in a while or features are seldom added.
> >>
> >> Exactly right.
> >>
> >> Although, it looks like M: doesn't even mean Maintainer.  If it did, I
> >> would have made these points over and over until death (or until I got
> >> bored).  However, as M: actually means "Mail patches to", there seems
> >> to be very little difference between that and "Designated reviewer"
> >> and makes me wonder why the R: tag was ever even introduced.  I guess
> >> all of the other guys in the threads below also thought M: meant
> >> Maintainer, or else they would have just added poor old Josh as a
> >> "Mail patches to" recipient and been done with it.
> >
> > Ah, but wait.  get_maintainer.pl *does* assume M means Maintainer
> > doesn't it?  Which is why this came about.  So if we have a "Mail
> > patches to" entry, get_maintainer.pl tells the user that this is a
> 
> Joe already answered but I'll elaborate a little bit:
> 
> "M:" means "Mail patches to" and "S:" means "Status" so what
> get_maintainers.pl is reports the person in "M:" printing the status
> of the file(s).
> 
> So for example if the file has "S: Supported" then the person listed
> in "M:" is shown as "supporter" while if the status is "S:
> Maintained", the person is listed as "maintainer".
> 
> There isn't a "Reviewed" status to specify files that are looked by
> "Designated reviewers", it can be added but I don't see the reason of
> it.

Not sure why you wrote all of this.  We know *why* get_maintainer.pl
does what it does.  What I'm saying is, that I personally believe this
is the wrong behaviour in what I'm *guessing* is the majority of the
time.

> > Maintainer, which (given that there are over 1000 unique M: entries
> > and I know that there are no where near that many actual Maintainers)
> > means that it's printing out incorrect information most of the time.
> 
> Well, that's correct according to your definition of maintainer
> (people with a tree that sends pull requests) but I can believe that
> there are 1K unique maintainers for different small components
> (without taking into account what components may be obsolete / not
> used anymore) even if these maintainers don't send pull request
> because the patch load is low and rely on subsystem maintainers to
> pick directly from the list with their Acks.

Then they are not Maintainers.  They are Reviewers who rely on
Maintainers.  In your example above it's the Maintainers that are
Maintainers and the people who review the code are Reviewers.  The
clues are in the words. ;)

> For me the maintainer is that a) cares about the file and makes sure
> that things remain working, fix issues, reviews patches from others,
> etc b) is someone that actually understand the code in the files. A
> subsystem maintainer has the last word of what gets merged into the
> subsystem but may not be familiar with code under the subsystem and
> relies on the file / driver maintainer to Ack the patch as correct
> since that person is the one that knows the code.

Then what is a Reviewer?

> > So back to my original thought then, what can we do to rectify this
> > situation and make the information printed more meaningful.  Again,
> > I'm back to using the R: tag appropriately.
> >
> > Any (technical, which aren't based on "but I really want to be listed
> > as a Maintainer") thoughts?
> >
> 
> I haven't read a "but I really want to be listed as a maintainer"
> thought in this thread.

If you had no vested interest in this, you would be able to see the
logic in what I'm saying.  Again I'm *guess*, but I think there is
some emotional reasons for you pushing back so hard.  I could always
be wrong though.

> I think the problem is the definition of what a maintainer in Linux
> really means. For you is someone that keeps a tree and sends pull
> request (for me that is what I call a subsystem maintainer)

Right.  A Level 3 Maintainer sends pull-requests to a Level 2
Maintainer, who sends pull-requests to a Level 1 Maintainer, who sends
pull-requests to Linus.  Maintainers at *all* levels collect patches
and Maintain them before sending on.

reiterate:

Reviewers care about particular driver(s)/domain(s) that they know
well.  They provide solid reviews and possible testing (because they
are likely to have the h/w).  They then provide Acked-by:s so that a
Maintainer can collect the patch. 

> while for
> others it seems that maintainer means someone who care about a set of
> files, knows the code, makes sure that things keep working and Ack
> patches to those files when posted.

        goto reiterate;   ;)

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Lee Jones]

On Wed, 28 Oct 2015, Krzysztof Kozlowski wrote:
> W dniu 28.10.2015 o 19:14, Lee Jones pisze:
> > On Wed, 28 Oct 2015, Krzysztof Kozlowski wrote:
> >> On 28.10.2015 17:24, Lee Jones wrote:
> >>> On Wed, 28 Oct 2015, Krzysztof Kozlowski wrote:
> >>>> 2015-10-28 3:44 GMT+09:00 Joe Perches <joe@perches.com>:
> >>>>> On Tue, 2015-10-27 at 18:15 +0000, Lee Jones wrote:
> >>>>>> On Tue, 27 Oct 2015, Sebastian Reichel wrote:> >
> >>>>>>> I think you should CC the people, which are changed from "M:"
> >>>>>>> to "R:", though.
> >>>>>>
> >>>>>> Yes, makes sense.
> >>>>>>
> >>>>>> I'd like to collect some Maintainer Acks first though.
> >>>>>
> >>>>> I think people from organizations like Samsung are actual 
> >>>>> maintainers not reviewers.
> >>>
> >>> So this all hinges on how we are describing Maintainers and
> >>> Reviewers.
> >>>
> >>> My personal definition (until convinced otherwise) is that Reviewers 
> >>> care about their particular subsystem and/or files.  They conduct
> >>> code reviews to ensure nothing gets broken and the code base stays in
> >>> best possible state of worthiness.  On the other hand Maintainers
> >>> usually conduct themselves as Reviewers but also have
> >>> 'maintainership' duties as well; such as applying patches,
> >>> *maintaining*, testing, rebasing, etc, an upstream branch and
> >>> ultimately sending pull-requests to higher level Maintainers i.e.
> >>> Linus.  Maintainers also have the ultimate say (unless over-ruled by
> >>> Linus etc) over what gets applied.
> >>
> >> Okay, sounds reasonable... so if a person performs reviews plus he does
> >> some of the other activities (not all) then who is he?
> > 
> > LT;DR: If someone doesn't *maintain* an upstream branch, they are not
> > an upstream Maintainer.
> 
> If I understand your point correctly: The maintainer and supporter
> should be mentioned if and only if he maintains an upstream branch?

Now we have the dedicated Reviewer tag, yes.  That's pretty much how I
see it.  Granted, before we had it Maintainer was the best term as
encompassed both the reviewing and actual maintaining roles, however
now there is a more informative tag which accurately describes the
reviewing role better.

I think Stephen said it best [0]:

"I don't care much whether it's "M:" or "R:", although "R:" carries
more meaning and hence is probably better."

> >> For example reviewing
> > 
> > Depends on the type of engagement.  Anyone can review any patch
> > submitted to anywhere on the code-base.  This does not make them an
> > accepted Reviewer.  Here I'm saying that a Reviewer is a competent
> > engineer who has made a promise to dedicate time to review incoming
> > patches in order to ensure quality.
> > 
> > To emphasise a tagged Reviewer isn't just someone who reviews code
> > every now and again.  It's someone who cares and has a vested interest
> > in either a subsystem as a whole, or perhaps individual or a group of
> > drivers.  However, this person does not conduct upstream branch
> > *maintenance*.
> > 
> >> testing + fixing bugs + cleaning up (sending
> >> patches from time to time)?
> > 
> > This is a Submitter/Contributor.
> > 
> > When I said testing before, I meant the branch being maintained, not
> > the driver on it's own.  That should be tested by Submitters/
> > Contributors or Testers (who get to provide their Tested-by).
> > 
> >> Would that be sufficient requirement to call him maintainer of a driver?
> >> Or maybe all of these requirements must be met (including handling of
> >> patches and sending pull reqs)?
> > 
> > It's only really the handing of patches and the maintenance of an
> > upstream branch which differentiates a Reviewer from a Maintainer. 
> > 
> >>>>> Their drivers are not thrown over a wall and forgotten.
> >>>>
> >>>> At least for Samsung Multifunction PMIC drivers (and some of Maxim 
> >>>> MUICs and PMICs) these are actively used by us in existing and new 
> >>>> products. They are also continuously extended and actually
> >>>> maintained. This means that it is not only about review of new
> >>>> patches but also about caring that nothing will become broken.
> >>>
> >>> Exactly.  This what I expect of any good code Reviewer.
> >>>
> >>>> I would prefer to leave the "SAMSUNG MULTIFUNCTION PMIC DEVICE 
> >>>> DRIVERS" entry as is - maintainers.
> >>>
> >>> But you aren't maintaining the driver i.e. you don't collect patches 
> >>> and *maintain* them on an upstream branch.
> >>
> >> Indeed, we don't. However are other non-reviewing activities sufficient?
> > 
> > The other non-reviewing activities you mentioned are that of the
> > Submitter/Contributor/Tester.  They still don't make someone a
> > Maintainer.
> 
> I think I got your point of view. I don't see it that way, especially
> that I pointed the fact of combining these activities.
> Submitter/Reviewer/Tester in one person.

Each of these are perfectly valid and extremely worthy roles.  They
all have their own way of being identified using Signed-off-by,
Reviewed-by, Tested-by tags and one of them 'Reviewer' even gets
mentioned in MAINTAINERS, but even if someone does all of them, that
doesn't make them a Maintainer.

> >>> Granted some of you guys 
> >>> are doing a great job of maintaining branches on your downstream or 
> >>> BSP kernels, but conduct a Reviewer type role for upstream.
> >>
> >> You mentioned also the "ultimate say over what gets applied" - which in
> >> this particular case is interesting to us because we have direct
> >> interest in these drivers being in a good shape and doing things we
> >> expect them to do. Like representing the interest of users.
> > 
> > Any Reviewers opinion will matter to someone who ultimately applies
> > the patches.  For instance, if you were to say to me "this change to
> > our MFD doesn't suit us because of X", I almost certainly won't apply
> > the patch.
> > 
> >> Of course one could say that every upstreaming person has such
> >> expectations... but some of the upstreamers just send a driver for one
> >> device. Or extend driver for one device. In this case this is a family
> >> of devices used on all of our Exynos SoC products and we care about all
> >> of them.
> > 
> > And being a nominated Reviewer mentioned in MAINTAINERS, that's
> > exactly what I'd expect.  If people fail to mean these requirement
> > they should be removed completely.
> 
> Both of the points above make sense. The person mentioned as reviewer
> should review.

Right. And know the code base and care about it and all of the other
things previously mentioned.

> >>> You guys are pushing back like this is some kind of demotion.
> >>> That's not the case at all.  All it does is better describe the (very
> >>> worthy) function you *actually* provide.
> >>
> >> It is getting into dispute about entire change of yours... which is not
> >> what I want. I agree with your general idea but I was referring only to
> >> that particular case - the Samsung PMICs (and Maxim PMICs/MUICs which
> >> would fall into same category).
> > 
> > I hope that I've explained my view adequately above.  My view is also
> > carried out over the Samsung drivers.
> 
> Yes, you explained your point of view... and we can agree to disagree.
> :) In the same time I actually accept the fact that I am not the person
> with any kind of knowledge about kernel development process.
> 
> I think I also described what I am doing with the Samsung drivers so if
> that falls under "Reviewing" then I am entirely fine with it.

Personally I think it does.  But I'm not the only one with an opinion,
so we either need to get some wider consensus or let sleeping dogs
lie and accept that the situation isn't perfect, or describe the real
situation adequately.

[0] https://lkml.org/lkml/2014/6/2/619

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Lee Jones]

On Wed, 28 Oct 2015, Krzysztof Kozlowski wrote:
> W dniu 28.10.2015 o 19:14, Lee Jones pisze:
> > On Wed, 28 Oct 2015, Krzysztof Kozlowski wrote:
> >> On 28.10.2015 17:24, Lee Jones wrote:
> >>> On Wed, 28 Oct 2015, Krzysztof Kozlowski wrote:
> >>>> 2015-10-28 3:44 GMT+09:00 Joe Perches <joe@perches.com>:
> >>>>> On Tue, 2015-10-27 at 18:15 +0000, Lee Jones wrote:
> >>>>>> On Tue, 27 Oct 2015, Sebastian Reichel wrote:> >
> >>>>>>> I think you should CC the people, which are changed from "M:"
> >>>>>>> to "R:", though.
> >>>>>>
> >>>>>> Yes, makes sense.
> >>>>>>
> >>>>>> I'd like to collect some Maintainer Acks first though.
> >>>>>
> >>>>> I think people from organizations like Samsung are actual 
> >>>>> maintainers not reviewers.
> >>>
> >>> So this all hinges on how we are describing Maintainers and
> >>> Reviewers.
> >>>
> >>> My personal definition (until convinced otherwise) is that Reviewers 
> >>> care about their particular subsystem and/or files.  They conduct
> >>> code reviews to ensure nothing gets broken and the code base stays in
> >>> best possible state of worthiness.  On the other hand Maintainers
> >>> usually conduct themselves as Reviewers but also have
> >>> 'maintainership' duties as well; such as applying patches,
> >>> *maintaining*, testing, rebasing, etc, an upstream branch and
> >>> ultimately sending pull-requests to higher level Maintainers i.e.
> >>> Linus.  Maintainers also have the ultimate say (unless over-ruled by
> >>> Linus etc) over what gets applied.
> >>
> >> Okay, sounds reasonable... so if a person performs reviews plus he does
> >> some of the other activities (not all) then who is he?
> > 
> > LT;DR: If someone doesn't *maintain* an upstream branch, they are not
> > an upstream Maintainer.
> 
> If I understand your point correctly: The maintainer and supporter
> should be mentioned if and only if he maintains an upstream branch?

Now we have the dedicated Reviewer tag, yes.  That's pretty much how I
see it.  Granted, before we had it Maintainer was the best term as
encompassed both the reviewing and actual maintaining roles, however
now there is a more informative tag which accurately describes the
reviewing role better.

I think Stephen said it best [0]:

"I don't care much whether it's "M:" or "R:", although "R:" carries
more meaning and hence is probably better."

> >> For example reviewing
> > 
> > Depends on the type of engagement.  Anyone can review any patch
> > submitted to anywhere on the code-base.  This does not make them an
> > accepted Reviewer.  Here I'm saying that a Reviewer is a competent
> > engineer who has made a promise to dedicate time to review incoming
> > patches in order to ensure quality.
> > 
> > To emphasise a tagged Reviewer isn't just someone who reviews code
> > every now and again.  It's someone who cares and has a vested interest
> > in either a subsystem as a whole, or perhaps individual or a group of
> > drivers.  However, this person does not conduct upstream branch
> > *maintenance*.
> > 
> >> testing + fixing bugs + cleaning up (sending
> >> patches from time to time)?
> > 
> > This is a Submitter/Contributor.
> > 
> > When I said testing before, I meant the branch being maintained, not
> > the driver on it's own.  That should be tested by Submitters/
> > Contributors or Testers (who get to provide their Tested-by).
> > 
> >> Would that be sufficient requirement to call him maintainer of a driver?
> >> Or maybe all of these requirements must be met (including handling of
> >> patches and sending pull reqs)?
> > 
> > It's only really the handing of patches and the maintenance of an
> > upstream branch which differentiates a Reviewer from a Maintainer. 
> > 
> >>>>> Their drivers are not thrown over a wall and forgotten.
> >>>>
> >>>> At least for Samsung Multifunction PMIC drivers (and some of Maxim 
> >>>> MUICs and PMICs) these are actively used by us in existing and new 
> >>>> products. They are also continuously extended and actually
> >>>> maintained. This means that it is not only about review of new
> >>>> patches but also about caring that nothing will become broken.
> >>>
> >>> Exactly.  This what I expect of any good code Reviewer.
> >>>
> >>>> I would prefer to leave the "SAMSUNG MULTIFUNCTION PMIC DEVICE 
> >>>> DRIVERS" entry as is - maintainers.
> >>>
> >>> But you aren't maintaining the driver i.e. you don't collect patches 
> >>> and *maintain* them on an upstream branch.
> >>
> >> Indeed, we don't. However are other non-reviewing activities sufficient?
> > 
> > The other non-reviewing activities you mentioned are that of the
> > Submitter/Contributor/Tester.  They still don't make someone a
> > Maintainer.
> 
> I think I got your point of view. I don't see it that way, especially
> that I pointed the fact of combining these activities.
> Submitter/Reviewer/Tester in one person.

Each of these are perfectly valid and extremely worthy roles.  They
all have their own way of being identified using Signed-off-by,
Reviewed-by, Tested-by tags and one of them 'Reviewer' even gets
mentioned in MAINTAINERS, but even if someone does all of them, that
doesn't make them a Maintainer.

> >>> Granted some of you guys 
> >>> are doing a great job of maintaining branches on your downstream or 
> >>> BSP kernels, but conduct a Reviewer type role for upstream.
> >>
> >> You mentioned also the "ultimate say over what gets applied" - which in
> >> this particular case is interesting to us because we have direct
> >> interest in these drivers being in a good shape and doing things we
> >> expect them to do. Like representing the interest of users.
> > 
> > Any Reviewers opinion will matter to someone who ultimately applies
> > the patches.  For instance, if you were to say to me "this change to
> > our MFD doesn't suit us because of X", I almost certainly won't apply
> > the patch.
> > 
> >> Of course one could say that every upstreaming person has such
> >> expectations... but some of the upstreamers just send a driver for one
> >> device. Or extend driver for one device. In this case this is a family
> >> of devices used on all of our Exynos SoC products and we care about all
> >> of them.
> > 
> > And being a nominated Reviewer mentioned in MAINTAINERS, that's
> > exactly what I'd expect.  If people fail to mean these requirement
> > they should be removed completely.
> 
> Both of the points above make sense. The person mentioned as reviewer
> should review.

Right. And know the code base and care about it and all of the other
things previously mentioned.

> >>> You guys are pushing back like this is some kind of demotion.
> >>> That's not the case at all.  All it does is better describe the (very
> >>> worthy) function you *actually* provide.
> >>
> >> It is getting into dispute about entire change of yours... which is not
> >> what I want. I agree with your general idea but I was referring only to
> >> that particular case - the Samsung PMICs (and Maxim PMICs/MUICs which
> >> would fall into same category).
> > 
> > I hope that I've explained my view adequately above.  My view is also
> > carried out over the Samsung drivers.
> 
> Yes, you explained your point of view... and we can agree to disagree.
> :) In the same time I actually accept the fact that I am not the person
> with any kind of knowledge about kernel development process.
> 
> I think I also described what I am doing with the Samsung drivers so if
> that falls under "Reviewing" then I am entirely fine with it.

Personally I think it does.  But I'm not the only one with an opinion,
so we either need to get some wider consensus or let sleeping dogs
lie and accept that the situation isn't perfect, or describe the real
situation adequately.

[0] https://lkml.org/lkml/2014/6/2/619

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Javier Martinez Canillas]

Hello Lee,

On Wed, Oct 28, 2015 at 2:34 PM, Lee Jones <lee.jones@linaro.org> wrote:
> On Wed, 28 Oct 2015, Javier Martinez Canillas wrote:
>> On Wed, Oct 28, 2015 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:
>> > On Wed, 28 Oct 2015, Lee Jones wrote:
>> >
>> >> On Wed, 28 Oct 2015, Javier Martinez Canillas wrote:
>> >>
>> >> > Hello Joe,
>> >> >
>> >> > On Wed, Oct 28, 2015 at 12:06 PM, Joe Perches <joe@perches.com> wrote:
>> >> > > On Wed, 2015-10-28 at 11:53 +0100, Javier Martinez Canillas wrote:
>> >> > >> (Lee) think(s) that the difference between a maintainer and
>> >> > >> a reviewer is if a branch with fixes / new features are kept and pull
>> >> > >> requests sent while I think that the difference is the level of
>> >> > >> involvement someone has with a driver regardless of how patches ends
>> >> > >> in the subsystem tree (picked directly by subsystem maintainers or
>> >> > >> sent through pull requests).
>> >> > >>
>> >> > >> Is the first time I heard your definition but maybe I'm the one that
>> >> > >> is wrong so it would be great to get a consensus on that and get it
>> >> > >> documented somewhere.
>> >> > >
>> >> > > I think Lee is over-analyzing.
>> >> > >
>> >> > > From MAINTAINERS:
>> >> > >         M: Mail patches to: FullName <address@domain>
>> >> > >         R: Designated reviewer: FullName <address@domain>
>> >> > >            These reviewers should be CCed on patches.
>> >> > >         S: Status, one of the following:
>> >> > >            Supported:   Someone is actually paid to look after this.
>> >> > >            Maintained:  Someone actually looks after it.
>> >> > >
>> >> > > "looking after" doesn't mean upstreaming.
>> >> > >
>> >> >
>> >> > Agreed and upstreaming doesn't mean sending pull request, you can for
>> >> > example upstream the downstream changes for a driver you maintain by
>> >> > posting patches or ack patches others post and let the subsystem
>> >> > maintainer to pick those (even if you are listed as the driver
>> >> > maintainer in MAINTAINERS).
>> >> >
>> >> > So by following Lee's definition, then most drivers' maintainers
>> >> > should not be called maintainers since keeping a tree with patches for
>> >> > both fixes and new features, sending pull requests, etc is only
>> >> > justified for drivers that have a lot of changes per release. Is not
>> >> > worth it for drivers that are in "maintenance mode" where only bugs
>> >> > are fixed every once in a while or features are seldom added.
>> >>
>> >> Exactly right.
>> >>
>> >> Although, it looks like M: doesn't even mean Maintainer.  If it did, I
>> >> would have made these points over and over until death (or until I got
>> >> bored).  However, as M: actually means "Mail patches to", there seems
>> >> to be very little difference between that and "Designated reviewer"
>> >> and makes me wonder why the R: tag was ever even introduced.  I guess
>> >> all of the other guys in the threads below also thought M: meant
>> >> Maintainer, or else they would have just added poor old Josh as a
>> >> "Mail patches to" recipient and been done with it.
>> >
>> > Ah, but wait.  get_maintainer.pl *does* assume M means Maintainer
>> > doesn't it?  Which is why this came about.  So if we have a "Mail
>> > patches to" entry, get_maintainer.pl tells the user that this is a
>>
>> Joe already answered but I'll elaborate a little bit:
>>
>> "M:" means "Mail patches to" and "S:" means "Status" so what
>> get_maintainers.pl is reports the person in "M:" printing the status
>> of the file(s).
>>
>> So for example if the file has "S: Supported" then the person listed
>> in "M:" is shown as "supporter" while if the status is "S:
>> Maintained", the person is listed as "maintainer".
>>
>> There isn't a "Reviewed" status to specify files that are looked by
>> "Designated reviewers", it can be added but I don't see the reason of
>> it.
>
> Not sure why you wrote all of this.  We know *why* get_maintainer.pl
> does what it does.  What I'm saying is, that I personally believe this
> is the wrong behaviour in what I'm *guessing* is the majority of the
> time.
>

Ok sorry if it was clear to you already.

>> > Maintainer, which (given that there are over 1000 unique M: entries
>> > and I know that there are no where near that many actual Maintainers)
>> > means that it's printing out incorrect information most of the time.
>>
>> Well, that's correct according to your definition of maintainer
>> (people with a tree that sends pull requests) but I can believe that
>> there are 1K unique maintainers for different small components
>> (without taking into account what components may be obsolete / not
>> used anymore) even if these maintainers don't send pull request
>> because the patch load is low and rely on subsystem maintainers to
>> pick directly from the list with their Acks.
>
> Then they are not Maintainers.  They are Reviewers who rely on
> Maintainers.  In your example above it's the Maintainers that are
> Maintainers and the people who review the code are Reviewers.  The
> clues are in the words. ;)
>

They are not maintainers according to your definition of maintainer
that doesn't seem what most people agree with.

>> For me the maintainer is that a) cares about the file and makes sure
>> that things remain working, fix issues, reviews patches from others,
>> etc b) is someone that actually understand the code in the files. A
>> subsystem maintainer has the last word of what gets merged into the
>> subsystem but may not be familiar with code under the subsystem and
>> relies on the file / driver maintainer to Ack the patch as correct
>> since that person is the one that knows the code.
>
> Then what is a Reviewer?
>

As I mentioned before, I think a reviewer is someone who is interested
in a given file / driver / subsystem and reviews patches to the list
but does not have an authoritative answer on that file. I gave as an
example that I review patches posted for Exynos SoC support although
I'm not mentioned in MAINTAINERS and I don't plan it to be. People
don't know that I'm interested in that and don't cc me, I just review
what is posted to the linux-samsung-soc ML.

A designated reviewer is someone that is mentioned in the MAINTAINERS
file so people know that this person is interested and will add
him/her as cc. So designated reviewers are expected to review more
patches, be more available, etc and it seems the idea is that they can
become maintainers in the future if there is some need for it.

>> > So back to my original thought then, what can we do to rectify this
>> > situation and make the information printed more meaningful.  Again,
>> > I'm back to using the R: tag appropriately.
>> >
>> > Any (technical, which aren't based on "but I really want to be listed
>> > as a Maintainer") thoughts?
>> >
>>
>> I haven't read a "but I really want to be listed as a maintainer"
>> thought in this thread.
>
> If you had no vested interest in this, you would be able to see the
> logic in what I'm saying.  Again I'm *guess*, but I think there is
> some emotional reasons for you pushing back so hard.  I could always
> be wrong though.
>

Oh, I have no personal interest. I'm not even listed as a maintainer /
reviewer for anything that is MFD related and I do understand your
point is just that I don't agree. The fact that we disagree doesn't
mean that you are right and I'm wrong.

So if anything I think this thread is more about "but I really don't
want people without a tree to be listed as a Maintainer".

What I do care is about consistency across different subystems since
the kernel development process is already complicated and there are
already differences in the workflow of the maintainers so people
contributing to many subsystems have to first do some research to
learn all the unwritten rules and conventions of a given susbystem and
subsystem maintainer before posting patches.

That's why adding the semantics of maintainers and reviewers to the
mix of things to keep in mind is not something that thrills me.

>> I think the problem is the definition of what a maintainer in Linux
>> really means. For you is someone that keeps a tree and sends pull
>> request (for me that is what I call a subsystem maintainer)
>
> Right.  A Level 3 Maintainer sends pull-requests to a Level 2
> Maintainer, who sends pull-requests to a Level 1 Maintainer, who sends
> pull-requests to Linus.  Maintainers at *all* levels collect patches
> and Maintain them before sending on.
>

Here is where we disagree (and it seems is not only me), someone who
acks patches posted to some files even if those are later picked for
someone higher in the maintenance hierarchy could be named maintainers
as well since they "own" those files and their answer should be
authoritative (no matter how the patches end into mainline).

At the end of the day is to make easier for developers to understand
who are the interested parties for a given change. It is much easier
to think in terms of different levels of maintainers that should be
copied so they can review and possibly people outside of that group if
they are subscribed to the mailing lists. Naming the first levels
reviewers makes no sense for me since it will only confuse new comers
IMHO.

> reiterate:
>
> Reviewers care about particular driver(s)/domain(s) that they know
> well.  They provide solid reviews and possible testing (because they
> are likely to have the h/w).  They then provide Acked-by:s so that a
> Maintainer can collect the patch.
>
>> while for
>> others it seems that maintainer means someone who care about a set of
>> files, knows the code, makes sure that things keep working and Ack
>> patches to those files when posted.
>
>         goto reiterate;   ;)
>

No need for that, I already explained my point of view several times
and you just think I'm wrong because I don't agree with you. So let's
just agree on disagree ;-)

Best regards,
Javier

[PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Javier Martinez Canillas]

Hello Lee,

On Wed, Oct 28, 2015 at 2:34 PM, Lee Jones <lee.jones@linaro.org> wrote:
> On Wed, 28 Oct 2015, Javier Martinez Canillas wrote:
>> On Wed, Oct 28, 2015 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:
>> > On Wed, 28 Oct 2015, Lee Jones wrote:
>> >
>> >> On Wed, 28 Oct 2015, Javier Martinez Canillas wrote:
>> >>
>> >> > Hello Joe,
>> >> >
>> >> > On Wed, Oct 28, 2015 at 12:06 PM, Joe Perches <joe@perches.com> wrote:
>> >> > > On Wed, 2015-10-28 at 11:53 +0100, Javier Martinez Canillas wrote:
>> >> > >> (Lee) think(s) that the difference between a maintainer and
>> >> > >> a reviewer is if a branch with fixes / new features are kept and pull
>> >> > >> requests sent while I think that the difference is the level of
>> >> > >> involvement someone has with a driver regardless of how patches ends
>> >> > >> in the subsystem tree (picked directly by subsystem maintainers or
>> >> > >> sent through pull requests).
>> >> > >>
>> >> > >> Is the first time I heard your definition but maybe I'm the one that
>> >> > >> is wrong so it would be great to get a consensus on that and get it
>> >> > >> documented somewhere.
>> >> > >
>> >> > > I think Lee is over-analyzing.
>> >> > >
>> >> > > From MAINTAINERS:
>> >> > >         M: Mail patches to: FullName <address@domain>
>> >> > >         R: Designated reviewer: FullName <address@domain>
>> >> > >            These reviewers should be CCed on patches.
>> >> > >         S: Status, one of the following:
>> >> > >            Supported:   Someone is actually paid to look after this.
>> >> > >            Maintained:  Someone actually looks after it.
>> >> > >
>> >> > > "looking after" doesn't mean upstreaming.
>> >> > >
>> >> >
>> >> > Agreed and upstreaming doesn't mean sending pull request, you can for
>> >> > example upstream the downstream changes for a driver you maintain by
>> >> > posting patches or ack patches others post and let the subsystem
>> >> > maintainer to pick those (even if you are listed as the driver
>> >> > maintainer in MAINTAINERS).
>> >> >
>> >> > So by following Lee's definition, then most drivers' maintainers
>> >> > should not be called maintainers since keeping a tree with patches for
>> >> > both fixes and new features, sending pull requests, etc is only
>> >> > justified for drivers that have a lot of changes per release. Is not
>> >> > worth it for drivers that are in "maintenance mode" where only bugs
>> >> > are fixed every once in a while or features are seldom added.
>> >>
>> >> Exactly right.
>> >>
>> >> Although, it looks like M: doesn't even mean Maintainer.  If it did, I
>> >> would have made these points over and over until death (or until I got
>> >> bored).  However, as M: actually means "Mail patches to", there seems
>> >> to be very little difference between that and "Designated reviewer"
>> >> and makes me wonder why the R: tag was ever even introduced.  I guess
>> >> all of the other guys in the threads below also thought M: meant
>> >> Maintainer, or else they would have just added poor old Josh as a
>> >> "Mail patches to" recipient and been done with it.
>> >
>> > Ah, but wait.  get_maintainer.pl *does* assume M means Maintainer
>> > doesn't it?  Which is why this came about.  So if we have a "Mail
>> > patches to" entry, get_maintainer.pl tells the user that this is a
>>
>> Joe already answered but I'll elaborate a little bit:
>>
>> "M:" means "Mail patches to" and "S:" means "Status" so what
>> get_maintainers.pl is reports the person in "M:" printing the status
>> of the file(s).
>>
>> So for example if the file has "S: Supported" then the person listed
>> in "M:" is shown as "supporter" while if the status is "S:
>> Maintained", the person is listed as "maintainer".
>>
>> There isn't a "Reviewed" status to specify files that are looked by
>> "Designated reviewers", it can be added but I don't see the reason of
>> it.
>
> Not sure why you wrote all of this.  We know *why* get_maintainer.pl
> does what it does.  What I'm saying is, that I personally believe this
> is the wrong behaviour in what I'm *guessing* is the majority of the
> time.
>

Ok sorry if it was clear to you already.

>> > Maintainer, which (given that there are over 1000 unique M: entries
>> > and I know that there are no where near that many actual Maintainers)
>> > means that it's printing out incorrect information most of the time.
>>
>> Well, that's correct according to your definition of maintainer
>> (people with a tree that sends pull requests) but I can believe that
>> there are 1K unique maintainers for different small components
>> (without taking into account what components may be obsolete / not
>> used anymore) even if these maintainers don't send pull request
>> because the patch load is low and rely on subsystem maintainers to
>> pick directly from the list with their Acks.
>
> Then they are not Maintainers.  They are Reviewers who rely on
> Maintainers.  In your example above it's the Maintainers that are
> Maintainers and the people who review the code are Reviewers.  The
> clues are in the words. ;)
>

They are not maintainers according to your definition of maintainer
that doesn't seem what most people agree with.

>> For me the maintainer is that a) cares about the file and makes sure
>> that things remain working, fix issues, reviews patches from others,
>> etc b) is someone that actually understand the code in the files. A
>> subsystem maintainer has the last word of what gets merged into the
>> subsystem but may not be familiar with code under the subsystem and
>> relies on the file / driver maintainer to Ack the patch as correct
>> since that person is the one that knows the code.
>
> Then what is a Reviewer?
>

As I mentioned before, I think a reviewer is someone who is interested
in a given file / driver / subsystem and reviews patches to the list
but does not have an authoritative answer on that file. I gave as an
example that I review patches posted for Exynos SoC support although
I'm not mentioned in MAINTAINERS and I don't plan it to be. People
don't know that I'm interested in that and don't cc me, I just review
what is posted to the linux-samsung-soc ML.

A designated reviewer is someone that is mentioned in the MAINTAINERS
file so people know that this person is interested and will add
him/her as cc. So designated reviewers are expected to review more
patches, be more available, etc and it seems the idea is that they can
become maintainers in the future if there is some need for it.

>> > So back to my original thought then, what can we do to rectify this
>> > situation and make the information printed more meaningful.  Again,
>> > I'm back to using the R: tag appropriately.
>> >
>> > Any (technical, which aren't based on "but I really want to be listed
>> > as a Maintainer") thoughts?
>> >
>>
>> I haven't read a "but I really want to be listed as a maintainer"
>> thought in this thread.
>
> If you had no vested interest in this, you would be able to see the
> logic in what I'm saying.  Again I'm *guess*, but I think there is
> some emotional reasons for you pushing back so hard.  I could always
> be wrong though.
>

Oh, I have no personal interest. I'm not even listed as a maintainer /
reviewer for anything that is MFD related and I do understand your
point is just that I don't agree. The fact that we disagree doesn't
mean that you are right and I'm wrong.

So if anything I think this thread is more about "but I really don't
want people without a tree to be listed as a Maintainer".

What I do care is about consistency across different subystems since
the kernel development process is already complicated and there are
already differences in the workflow of the maintainers so people
contributing to many subsystems have to first do some research to
learn all the unwritten rules and conventions of a given susbystem and
subsystem maintainer before posting patches.

That's why adding the semantics of maintainers and reviewers to the
mix of things to keep in mind is not something that thrills me.

>> I think the problem is the definition of what a maintainer in Linux
>> really means. For you is someone that keeps a tree and sends pull
>> request (for me that is what I call a subsystem maintainer)
>
> Right.  A Level 3 Maintainer sends pull-requests to a Level 2
> Maintainer, who sends pull-requests to a Level 1 Maintainer, who sends
> pull-requests to Linus.  Maintainers at *all* levels collect patches
> and Maintain them before sending on.
>

Here is where we disagree (and it seems is not only me), someone who
acks patches posted to some files even if those are later picked for
someone higher in the maintenance hierarchy could be named maintainers
as well since they "own" those files and their answer should be
authoritative (no matter how the patches end into mainline).

At the end of the day is to make easier for developers to understand
who are the interested parties for a given change. It is much easier
to think in terms of different levels of maintainers that should be
copied so they can review and possibly people outside of that group if
they are subscribed to the mailing lists. Naming the first levels
reviewers makes no sense for me since it will only confuse new comers
IMHO.

> reiterate:
>
> Reviewers care about particular driver(s)/domain(s) that they know
> well.  They provide solid reviews and possible testing (because they
> are likely to have the h/w).  They then provide Acked-by:s so that a
> Maintainer can collect the patch.
>
>> while for
>> others it seems that maintainer means someone who care about a set of
>> files, knows the code, makes sure that things keep working and Ack
>> patches to those files when posted.
>
>         goto reiterate;   ;)
>

No need for that, I already explained my point of view several times
and you just think I'm wrong because I don't agree with you. So let's
just agree on disagree ;-)

Best regards,
Javier

Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Lee Jones]

On Wed, 28 Oct 2015, Javier Martinez Canillas wrote:
> On Wed, Oct 28, 2015 at 2:34 PM, Lee Jones <lee.jones@linaro.org> wrote:
> > On Wed, 28 Oct 2015, Javier Martinez Canillas wrote:
> >> On Wed, Oct 28, 2015 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:
> >> > On Wed, 28 Oct 2015, Lee Jones wrote:
> >> >
> >> >> On Wed, 28 Oct 2015, Javier Martinez Canillas wrote:
> >> >>
> >> >> > Hello Joe,
> >> >> >
> >> >> > On Wed, Oct 28, 2015 at 12:06 PM, Joe Perches <joe@perches.com> wrote:
> >> >> > > On Wed, 2015-10-28 at 11:53 +0100, Javier Martinez Canillas wrote:
> >> >> > >> (Lee) think(s) that the difference between a maintainer and
> >> >> > >> a reviewer is if a branch with fixes / new features are kept and pull
> >> >> > >> requests sent while I think that the difference is the level of
> >> >> > >> involvement someone has with a driver regardless of how patches ends
> >> >> > >> in the subsystem tree (picked directly by subsystem maintainers or
> >> >> > >> sent through pull requests).
> >> >> > >>
> >> >> > >> Is the first time I heard your definition but maybe I'm the one that
> >> >> > >> is wrong so it would be great to get a consensus on that and get it
> >> >> > >> documented somewhere.
> >> >> > >
> >> >> > > I think Lee is over-analyzing.
> >> >> > >
> >> >> > > From MAINTAINERS:
> >> >> > >         M: Mail patches to: FullName <address@domain>
> >> >> > >         R: Designated reviewer: FullName <address@domain>
> >> >> > >            These reviewers should be CCed on patches.
> >> >> > >         S: Status, one of the following:
> >> >> > >            Supported:   Someone is actually paid to look after this.
> >> >> > >            Maintained:  Someone actually looks after it.
> >> >> > >
> >> >> > > "looking after" doesn't mean upstreaming.
> >> >> > >
> >> >> >
> >> >> > Agreed and upstreaming doesn't mean sending pull request, you can for
> >> >> > example upstream the downstream changes for a driver you maintain by
> >> >> > posting patches or ack patches others post and let the subsystem
> >> >> > maintainer to pick those (even if you are listed as the driver
> >> >> > maintainer in MAINTAINERS).
> >> >> >
> >> >> > So by following Lee's definition, then most drivers' maintainers
> >> >> > should not be called maintainers since keeping a tree with patches for
> >> >> > both fixes and new features, sending pull requests, etc is only
> >> >> > justified for drivers that have a lot of changes per release. Is not
> >> >> > worth it for drivers that are in "maintenance mode" where only bugs
> >> >> > are fixed every once in a while or features are seldom added.
> >> >>
> >> >> Exactly right.
> >> >>
> >> >> Although, it looks like M: doesn't even mean Maintainer.  If it did, I
> >> >> would have made these points over and over until death (or until I got
> >> >> bored).  However, as M: actually means "Mail patches to", there seems
> >> >> to be very little difference between that and "Designated reviewer"
> >> >> and makes me wonder why the R: tag was ever even introduced.  I guess
> >> >> all of the other guys in the threads below also thought M: meant
> >> >> Maintainer, or else they would have just added poor old Josh as a
> >> >> "Mail patches to" recipient and been done with it.
> >> >
> >> > Ah, but wait.  get_maintainer.pl *does* assume M means Maintainer
> >> > doesn't it?  Which is why this came about.  So if we have a "Mail
> >> > patches to" entry, get_maintainer.pl tells the user that this is a
> >>
> >> Joe already answered but I'll elaborate a little bit:
> >>
> >> "M:" means "Mail patches to" and "S:" means "Status" so what
> >> get_maintainers.pl is reports the person in "M:" printing the status
> >> of the file(s).
> >>
> >> So for example if the file has "S: Supported" then the person listed
> >> in "M:" is shown as "supporter" while if the status is "S:
> >> Maintained", the person is listed as "maintainer".
> >>
> >> There isn't a "Reviewed" status to specify files that are looked by
> >> "Designated reviewers", it can be added but I don't see the reason of
> >> it.
> >
> > Not sure why you wrote all of this.  We know *why* get_maintainer.pl
> > does what it does.  What I'm saying is, that I personally believe this
> > is the wrong behaviour in what I'm *guessing* is the majority of the
> > time.
> 
> Ok sorry if it was clear to you already.

NP.

> >> > Maintainer, which (given that there are over 1000 unique M: entries
> >> > and I know that there are no where near that many actual Maintainers)
> >> > means that it's printing out incorrect information most of the time.
> >>
> >> Well, that's correct according to your definition of maintainer
> >> (people with a tree that sends pull requests) but I can believe that
> >> there are 1K unique maintainers for different small components
> >> (without taking into account what components may be obsolete / not
> >> used anymore) even if these maintainers don't send pull request
> >> because the patch load is low and rely on subsystem maintainers to
> >> pick directly from the list with their Acks.
> >
> > Then they are not Maintainers.  They are Reviewers who rely on
> > Maintainers.  In your example above it's the Maintainers that are
> > Maintainers and the people who review the code are Reviewers.  The
> > clues are in the words. ;)
> 
> They are not maintainers according to your definition of maintainer
> that doesn't seem what most people agree with.

"most people" so far are 3 people that I assume still want to be
Maintainers despite not actually conducting Maintainer duties, but
are rather Reviewers.  I also have 2 Acks for this patch, so thus far
that's 3 that agree and 3 that do not.  Unsurprisingly the ones that
agree are Maintainers and the ones who are not are (by my definition)
Reviewers -- go figure.

> >> For me the maintainer is that a) cares about the file and makes sure
> >> that things remain working, fix issues, reviews patches from others,
> >> etc b) is someone that actually understand the code in the files. A
> >> subsystem maintainer has the last word of what gets merged into the
> >> subsystem but may not be familiar with code under the subsystem and
> >> relies on the file / driver maintainer to Ack the patch as correct
> >> since that person is the one that knows the code.
> >
> > Then what is a Reviewer?
> 
> As I mentioned before, I think a reviewer is someone who is interested
> in a given file / driver / subsystem and reviews patches to the list
> but does not have an authoritative answer on that file. I gave as an
> example that I review patches posted for Exynos SoC support although
> I'm not mentioned in MAINTAINERS and I don't plan it to be. People
> don't know that I'm interested in that and don't cc me, I just review
> what is posted to the linux-samsung-soc ML.

*sigh* I've explained this (to you directly) before.  What you explain
here is just someone that occasionally reviews code.  Anyone can do
that, it doesn't make them a "Designated reviewer" and doesn't warrant
listing in MAINTAINERS.  A Reviewer (note the capitalisation whenever
I use the term like this) is someone who has volunteered or has been
identified as someone to conduct all of the duties you've mentioned
before.

So my question still stands.  What is a Reviewer in the context of the
MAINTAINERS file.  I'm suggesting that this is the title you give
someone when they; know the code, care about the code, review changes
to the code and possibly test the code (if they have the h/w).  I also
suggest they to not apply patches, maintain an upstream branch and
submit pull-requests to Linus or another senior Maintainer.

> A designated reviewer is someone that is mentioned in the MAINTAINERS
> file so people know that this person is interested and will add
> him/her as cc. So designated reviewers are expected to review more
> patches, be more available, etc

Right.  So how is this any different to what you're proposing is
actually a Maintainer?  I suggest the traits are the same.

> and it seems the idea is that they can
> become maintainers in the future if there is some need for it.

You just made that bit up. ;)

> >> > So back to my original thought then, what can we do to rectify this
> >> > situation and make the information printed more meaningful.  Again,
> >> > I'm back to using the R: tag appropriately.
> >> >
> >> > Any (technical, which aren't based on "but I really want to be listed
> >> > as a Maintainer") thoughts?
> >> >
> >>
> >> I haven't read a "but I really want to be listed as a maintainer"
> >> thought in this thread.
> >
> > If you had no vested interest in this, you would be able to see the
> > logic in what I'm saying.  Again I'm *guess*, but I think there is
> > some emotional reasons for you pushing back so hard.  I could always
> > be wrong though.
> >
> 
> Oh, I have no personal interest. I'm not even listed as a maintainer /
> reviewer for anything that is MFD related and I do understand your
> point is just that I don't agree. The fact that we disagree doesn't
> mean that you are right and I'm wrong.

Nor visa versa. ;)

> So if anything I think this thread is more about "but I really don't
> want people without a tree to be listed as a Maintainer".

This is 'really' about Maintainers being listed as Maintainers and
Reviewers being listed as Reviewers.  It doesn't get any simple or
logical than that.  I have no emotional attachment to either my
reasoning nor the patch.  It just makes sense.

> What I do care is about consistency across different subystems since
> the kernel development process is already complicated and there are
> already differences in the workflow of the maintainers so people
> contributing to many subsystems have to first do some research to
> learn all the unwritten rules and conventions of a given susbystem and
> subsystem maintainer before posting patches.

Amen to that.  And wouldn't life be simpler if submitters knew who
is likely to review their code and who is likely to apply it?

> That's why adding the semantics of maintainers and reviewers to the
> mix of things to keep in mind is not something that thrills me.

The semantics are already there.  They've been accepted by some pretty
senior Maintainers already.  What we're doing here is just using those
accepted semantics.

> >> I think the problem is the definition of what a maintainer in Linux
> >> really means. For you is someone that keeps a tree and sends pull
> >> request (for me that is what I call a subsystem maintainer)
> >
> > Right.  A Level 3 Maintainer sends pull-requests to a Level 2
> > Maintainer, who sends pull-requests to a Level 1 Maintainer, who sends
> > pull-requests to Linus.  Maintainers at *all* levels collect patches
> > and Maintain them before sending on.
> >
> 
> Here is where we disagree (and it seems is not only me), someone who
> acks patches posted to some files even if those are later picked for
> someone higher in the maintenance hierarchy could be named maintainers
> as well since they "own" those files and their answer should be
> authoritative (no matter how the patches end into mainline).
> 
> At the end of the day is to make easier for developers to understand
> who are the interested parties for a given change. It is much easier
> to think in terms of different levels of maintainers that should be
> copied so they can review and possibly people outside of that group if
> they are subscribed to the mailing lists. Naming the first levels
> reviewers makes no sense for me since it will only confuse new comers
> IMHO.

There's no level of Reviewer.  They are not under the Maintainers in
any way.  They do a different role and serve a different purpose.  I
believe it's clearer to submitters if they know who the Reviewers are
compared with who will actually take the patch into the 'system'.

> > reiterate:
> >
> > Reviewers care about particular driver(s)/domain(s) that they know
> > well.  They provide solid reviews and possible testing (because they
> > are likely to have the h/w).  They then provide Acked-by:s so that a
> > Maintainer can collect the patch.
> >
> >> while for
> >> others it seems that maintainer means someone who care about a set of
> >> files, knows the code, makes sure that things keep working and Ack
> >> patches to those files when posted.
> >
> >         goto reiterate;   ;)
> >
> 
> No need for that, I already explained my point of view several times
> and you just think I'm wrong because I don't agree with you. So let's
> just agree on disagree ;-)

I do think you're wrong.  And you think I am wrong.  My reasoning is
based on logic, common sense and words, and yours is based on, well, I
really don't know. ;)

So yes, we'll have to agree to disagree or neither of us will get any
real work done today.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Lee Jones]

On Wed, 28 Oct 2015, Javier Martinez Canillas wrote:
> On Wed, Oct 28, 2015 at 2:34 PM, Lee Jones <lee.jones@linaro.org> wrote:
> > On Wed, 28 Oct 2015, Javier Martinez Canillas wrote:
> >> On Wed, Oct 28, 2015 at 1:14 PM, Lee Jones <lee.jones@linaro.org> wrote:
> >> > On Wed, 28 Oct 2015, Lee Jones wrote:
> >> >
> >> >> On Wed, 28 Oct 2015, Javier Martinez Canillas wrote:
> >> >>
> >> >> > Hello Joe,
> >> >> >
> >> >> > On Wed, Oct 28, 2015 at 12:06 PM, Joe Perches <joe@perches.com> wrote:
> >> >> > > On Wed, 2015-10-28 at 11:53 +0100, Javier Martinez Canillas wrote:
> >> >> > >> (Lee) think(s) that the difference between a maintainer and
> >> >> > >> a reviewer is if a branch with fixes / new features are kept and pull
> >> >> > >> requests sent while I think that the difference is the level of
> >> >> > >> involvement someone has with a driver regardless of how patches ends
> >> >> > >> in the subsystem tree (picked directly by subsystem maintainers or
> >> >> > >> sent through pull requests).
> >> >> > >>
> >> >> > >> Is the first time I heard your definition but maybe I'm the one that
> >> >> > >> is wrong so it would be great to get a consensus on that and get it
> >> >> > >> documented somewhere.
> >> >> > >
> >> >> > > I think Lee is over-analyzing.
> >> >> > >
> >> >> > > From MAINTAINERS:
> >> >> > >         M: Mail patches to: FullName <address@domain>
> >> >> > >         R: Designated reviewer: FullName <address@domain>
> >> >> > >            These reviewers should be CCed on patches.
> >> >> > >         S: Status, one of the following:
> >> >> > >            Supported:   Someone is actually paid to look after this.
> >> >> > >            Maintained:  Someone actually looks after it.
> >> >> > >
> >> >> > > "looking after" doesn't mean upstreaming.
> >> >> > >
> >> >> >
> >> >> > Agreed and upstreaming doesn't mean sending pull request, you can for
> >> >> > example upstream the downstream changes for a driver you maintain by
> >> >> > posting patches or ack patches others post and let the subsystem
> >> >> > maintainer to pick those (even if you are listed as the driver
> >> >> > maintainer in MAINTAINERS).
> >> >> >
> >> >> > So by following Lee's definition, then most drivers' maintainers
> >> >> > should not be called maintainers since keeping a tree with patches for
> >> >> > both fixes and new features, sending pull requests, etc is only
> >> >> > justified for drivers that have a lot of changes per release. Is not
> >> >> > worth it for drivers that are in "maintenance mode" where only bugs
> >> >> > are fixed every once in a while or features are seldom added.
> >> >>
> >> >> Exactly right.
> >> >>
> >> >> Although, it looks like M: doesn't even mean Maintainer.  If it did, I
> >> >> would have made these points over and over until death (or until I got
> >> >> bored).  However, as M: actually means "Mail patches to", there seems
> >> >> to be very little difference between that and "Designated reviewer"
> >> >> and makes me wonder why the R: tag was ever even introduced.  I guess
> >> >> all of the other guys in the threads below also thought M: meant
> >> >> Maintainer, or else they would have just added poor old Josh as a
> >> >> "Mail patches to" recipient and been done with it.
> >> >
> >> > Ah, but wait.  get_maintainer.pl *does* assume M means Maintainer
> >> > doesn't it?  Which is why this came about.  So if we have a "Mail
> >> > patches to" entry, get_maintainer.pl tells the user that this is a
> >>
> >> Joe already answered but I'll elaborate a little bit:
> >>
> >> "M:" means "Mail patches to" and "S:" means "Status" so what
> >> get_maintainers.pl is reports the person in "M:" printing the status
> >> of the file(s).
> >>
> >> So for example if the file has "S: Supported" then the person listed
> >> in "M:" is shown as "supporter" while if the status is "S:
> >> Maintained", the person is listed as "maintainer".
> >>
> >> There isn't a "Reviewed" status to specify files that are looked by
> >> "Designated reviewers", it can be added but I don't see the reason of
> >> it.
> >
> > Not sure why you wrote all of this.  We know *why* get_maintainer.pl
> > does what it does.  What I'm saying is, that I personally believe this
> > is the wrong behaviour in what I'm *guessing* is the majority of the
> > time.
> 
> Ok sorry if it was clear to you already.

NP.

> >> > Maintainer, which (given that there are over 1000 unique M: entries
> >> > and I know that there are no where near that many actual Maintainers)
> >> > means that it's printing out incorrect information most of the time.
> >>
> >> Well, that's correct according to your definition of maintainer
> >> (people with a tree that sends pull requests) but I can believe that
> >> there are 1K unique maintainers for different small components
> >> (without taking into account what components may be obsolete / not
> >> used anymore) even if these maintainers don't send pull request
> >> because the patch load is low and rely on subsystem maintainers to
> >> pick directly from the list with their Acks.
> >
> > Then they are not Maintainers.  They are Reviewers who rely on
> > Maintainers.  In your example above it's the Maintainers that are
> > Maintainers and the people who review the code are Reviewers.  The
> > clues are in the words. ;)
> 
> They are not maintainers according to your definition of maintainer
> that doesn't seem what most people agree with.

"most people" so far are 3 people that I assume still want to be
Maintainers despite not actually conducting Maintainer duties, but
are rather Reviewers.  I also have 2 Acks for this patch, so thus far
that's 3 that agree and 3 that do not.  Unsurprisingly the ones that
agree are Maintainers and the ones who are not are (by my definition)
Reviewers -- go figure.

> >> For me the maintainer is that a) cares about the file and makes sure
> >> that things remain working, fix issues, reviews patches from others,
> >> etc b) is someone that actually understand the code in the files. A
> >> subsystem maintainer has the last word of what gets merged into the
> >> subsystem but may not be familiar with code under the subsystem and
> >> relies on the file / driver maintainer to Ack the patch as correct
> >> since that person is the one that knows the code.
> >
> > Then what is a Reviewer?
> 
> As I mentioned before, I think a reviewer is someone who is interested
> in a given file / driver / subsystem and reviews patches to the list
> but does not have an authoritative answer on that file. I gave as an
> example that I review patches posted for Exynos SoC support although
> I'm not mentioned in MAINTAINERS and I don't plan it to be. People
> don't know that I'm interested in that and don't cc me, I just review
> what is posted to the linux-samsung-soc ML.

*sigh* I've explained this (to you directly) before.  What you explain
here is just someone that occasionally reviews code.  Anyone can do
that, it doesn't make them a "Designated reviewer" and doesn't warrant
listing in MAINTAINERS.  A Reviewer (note the capitalisation whenever
I use the term like this) is someone who has volunteered or has been
identified as someone to conduct all of the duties you've mentioned
before.

So my question still stands.  What is a Reviewer in the context of the
MAINTAINERS file.  I'm suggesting that this is the title you give
someone when they; know the code, care about the code, review changes
to the code and possibly test the code (if they have the h/w).  I also
suggest they to not apply patches, maintain an upstream branch and
submit pull-requests to Linus or another senior Maintainer.

> A designated reviewer is someone that is mentioned in the MAINTAINERS
> file so people know that this person is interested and will add
> him/her as cc. So designated reviewers are expected to review more
> patches, be more available, etc

Right.  So how is this any different to what you're proposing is
actually a Maintainer?  I suggest the traits are the same.

> and it seems the idea is that they can
> become maintainers in the future if there is some need for it.

You just made that bit up. ;)

> >> > So back to my original thought then, what can we do to rectify this
> >> > situation and make the information printed more meaningful.  Again,
> >> > I'm back to using the R: tag appropriately.
> >> >
> >> > Any (technical, which aren't based on "but I really want to be listed
> >> > as a Maintainer") thoughts?
> >> >
> >>
> >> I haven't read a "but I really want to be listed as a maintainer"
> >> thought in this thread.
> >
> > If you had no vested interest in this, you would be able to see the
> > logic in what I'm saying.  Again I'm *guess*, but I think there is
> > some emotional reasons for you pushing back so hard.  I could always
> > be wrong though.
> >
> 
> Oh, I have no personal interest. I'm not even listed as a maintainer /
> reviewer for anything that is MFD related and I do understand your
> point is just that I don't agree. The fact that we disagree doesn't
> mean that you are right and I'm wrong.

Nor visa versa. ;)

> So if anything I think this thread is more about "but I really don't
> want people without a tree to be listed as a Maintainer".

This is 'really' about Maintainers being listed as Maintainers and
Reviewers being listed as Reviewers.  It doesn't get any simple or
logical than that.  I have no emotional attachment to either my
reasoning nor the patch.  It just makes sense.

> What I do care is about consistency across different subystems since
> the kernel development process is already complicated and there are
> already differences in the workflow of the maintainers so people
> contributing to many subsystems have to first do some research to
> learn all the unwritten rules and conventions of a given susbystem and
> subsystem maintainer before posting patches.

Amen to that.  And wouldn't life be simpler if submitters knew who
is likely to review their code and who is likely to apply it?

> That's why adding the semantics of maintainers and reviewers to the
> mix of things to keep in mind is not something that thrills me.

The semantics are already there.  They've been accepted by some pretty
senior Maintainers already.  What we're doing here is just using those
accepted semantics.

> >> I think the problem is the definition of what a maintainer in Linux
> >> really means. For you is someone that keeps a tree and sends pull
> >> request (for me that is what I call a subsystem maintainer)
> >
> > Right.  A Level 3 Maintainer sends pull-requests to a Level 2
> > Maintainer, who sends pull-requests to a Level 1 Maintainer, who sends
> > pull-requests to Linus.  Maintainers at *all* levels collect patches
> > and Maintain them before sending on.
> >
> 
> Here is where we disagree (and it seems is not only me), someone who
> acks patches posted to some files even if those are later picked for
> someone higher in the maintenance hierarchy could be named maintainers
> as well since they "own" those files and their answer should be
> authoritative (no matter how the patches end into mainline).
> 
> At the end of the day is to make easier for developers to understand
> who are the interested parties for a given change. It is much easier
> to think in terms of different levels of maintainers that should be
> copied so they can review and possibly people outside of that group if
> they are subscribed to the mailing lists. Naming the first levels
> reviewers makes no sense for me since it will only confuse new comers
> IMHO.

There's no level of Reviewer.  They are not under the Maintainers in
any way.  They do a different role and serve a different purpose.  I
believe it's clearer to submitters if they know who the Reviewers are
compared with who will actually take the patch into the 'system'.

> > reiterate:
> >
> > Reviewers care about particular driver(s)/domain(s) that they know
> > well.  They provide solid reviews and possible testing (because they
> > are likely to have the h/w).  They then provide Acked-by:s so that a
> > Maintainer can collect the patch.
> >
> >> while for
> >> others it seems that maintainer means someone who care about a set of
> >> files, knows the code, makes sure that things keep working and Ack
> >> patches to those files when posted.
> >
> >         goto reiterate;   ;)
> >
> 
> No need for that, I already explained my point of view several times
> and you just think I'm wrong because I don't agree with you. So let's
> just agree on disagree ;-)

I do think you're wrong.  And you think I am wrong.  My reasoning is
based on logic, common sense and words, and yours is based on, well, I
really don't know. ;)

So yes, we'll have to agree to disagree or neither of us will get any
real work done today.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Javier Martinez Canillas]

Hello Lee,

[removed since we have been discussing the same back and forth]

On Wed, Oct 28, 2015 at 3:38 PM, Lee Jones <lee.jones@linaro.org> wrote:
>>
>> No need for that, I already explained my point of view several times
>> and you just think I'm wrong because I don't agree with you. So let's
>> just agree on disagree ;-)
>
> I do think you're wrong.  And you think I am wrong.  My reasoning is
> based on logic, common sense and words, and yours is based on, well, I
> really don't know. ;)
>

My reasoning is based on two things basically:

1) Simplicity: it is easier to think in terms of developers (that post
patches) and maintainers (that reviews / acks patches) instead of
developers (that post patches), reviewers (that review patches) and
maintainers (that collect patches and push them upstream).

Individual developers posting patches should not even care what is the
flow of a patch into mainline IMHO, just who should be in copy (which
should be both the "own" of the file whatever is called and the
subsystem maintainer).

2) Consistency: I think that the developer / maintainer split is what
more people are familiar with than the developer / reviewer /
maintainer split.

I don't really have a strong opinion and I'm completely OK if all the
people that don't keep a tree are renamed to Reviewer, Author,
Whatever instead of Maintainer. The only thing that I ask is if this
could be made consistently across all subsystems and have an agreement
in consensus so developers knows what the semantics are regardless of
what subsystems they need to contribute.

> So yes, we'll have to agree to disagree or neither of us will get any
> real work done today.
>

Agreed, I spent most of my morning on this thread ;)

Best regards,
Javier

[PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Javier Martinez Canillas]

Hello Lee,

[removed since we have been discussing the same back and forth]

On Wed, Oct 28, 2015 at 3:38 PM, Lee Jones <lee.jones@linaro.org> wrote:
>>
>> No need for that, I already explained my point of view several times
>> and you just think I'm wrong because I don't agree with you. So let's
>> just agree on disagree ;-)
>
> I do think you're wrong.  And you think I am wrong.  My reasoning is
> based on logic, common sense and words, and yours is based on, well, I
> really don't know. ;)
>

My reasoning is based on two things basically:

1) Simplicity: it is easier to think in terms of developers (that post
patches) and maintainers (that reviews / acks patches) instead of
developers (that post patches), reviewers (that review patches) and
maintainers (that collect patches and push them upstream).

Individual developers posting patches should not even care what is the
flow of a patch into mainline IMHO, just who should be in copy (which
should be both the "own" of the file whatever is called and the
subsystem maintainer).

2) Consistency: I think that the developer / maintainer split is what
more people are familiar with than the developer / reviewer /
maintainer split.

I don't really have a strong opinion and I'm completely OK if all the
people that don't keep a tree are renamed to Reviewer, Author,
Whatever instead of Maintainer. The only thing that I ask is if this
could be made consistently across all subsystems and have an agreement
in consensus so developers knows what the semantics are regardless of
what subsystems they need to contribute.

> So yes, we'll have to agree to disagree or neither of us will get any
> real work done today.
>

Agreed, I spent most of my morning on this thread ;)

Best regards,
Javier

Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Bartlomiej Zolnierkiewicz]

[ this time with full Cc: & context preserved ]

Hi,

On Wednesday, October 28, 2015 08:24:46 AM Lee Jones wrote:
> On Tue, 2015-10-27 at 18:15 +0000, Lee Jones wrote:
> > On Tue, 27 Oct 2015, Sebastian Reichel wrote:
> > > On Tue, Oct 27, 2015 at 03:42:37PM +0000, Lee Jones wrote:
> > > > Since eafbaac ("MAINTAINERS: Add "R:" designated-reviewers tag") we
> > > > have been able to tag specific people as Reviewers.  These are key
> > > > individuals who are tasked with or volunteer to review code submitted
> > > > to a subsystem or specific file.  However, according to MAINTAINERS
> > > > we have 1046 Maintainers and only a mere 22 Reviewers.  I believe
> > > > these numbers to be incorrect, as many of these Maintainers are in
> > > > fact Reviewers.
> 
> Most entries in MAINTAINERS seem to be vanity entries than actual
> active participants.  A person typically writes a driver, adds a
> MAINTAINER entry, then forgets about it and/or the hardware becomes
> outdated.
> 
> This I agree with.
> 
> On Wed, 28 Oct 2015, Krzysztof Kozlowski wrote:
> > 2015-10-28 3:44 GMT+09:00 Joe Perches <joe@perches.com>:
> > > On Tue, 2015-10-27 at 18:15 +0000, Lee Jones wrote:
> > > > On Tue, 27 Oct 2015, Sebastian Reichel wrote:> >
> > > > > I think you should CC the people, which are changed from "M:" to
> > > > > "R:", though.
> > > >
> > > > Yes, makes sense.
> > > >
> > > > I'd like to collect some Maintainer Acks first though.
> > >
> > > I think people from organizations like Samsung are actual
> > > maintainers not reviewers.
> 
> So this all hinges on how we are describing Maintainers and Reviewers.
> 
> My personal definition (until convinced otherwise) is that Reviewers
> care about their particular subsystem and/or files.  They conduct code
> reviews to ensure nothing gets broken and the code base stays in best
> possible state of worthiness.  On the other hand Maintainers usually
> conduct themselves as Reviewers but also have 'maintainership' duties
> as well; such as applying patches, *maintaining*, testing, rebasing,
> etc, an upstream branch and ultimately sending pull-requests to higher
> level Maintainers i.e. Linus.  Maintainers also have the ultimate say
> (unless over-ruled by Linus etc) over what gets applied.
> 
> > > Their drivers are not thrown over a wall and forgotten.
> > 
> > At least for Samsung Multifunction PMIC drivers (and some of Maxim
> > MUICs and PMICs) these are actively used by us in existing and new
> > products. They are also continuously extended and actually maintained.
> > This means that it is not only about review of new patches but also
> > about caring that nothing will become broken.
> 
> Exactly.  This what I expect of any good code Reviewer.
> 
> > I would prefer to leave the "SAMSUNG MULTIFUNCTION PMIC DEVICE
> > DRIVERS" entry as is - maintainers.
> 
> But you aren't maintaining the driver i.e. you don't collect patches
> and *maintain* them on an upstream branch.  Granted some of you guys
> are doing a great job of maintaining branches on your downstream or
> BSP kernels, but conduct a Reviewer type role for upstream.
> 
> You guys are pushing back like this is some kind of demotion.  That's
> not the case at all.  All it does is better describe the (very worthy)
> function you *actually* provide.

It is actually a demotion from my POV:

* "Reviewer" doesn't accurately describe the job of doing all the needed
  testing, bug-fixing and additional contributions that is often done by
  people without their own branches.

* You don't know internal policies of all companies involved in Linux
  Kernel development.  "Maintainer" is a well known term and sometimes
  person's job status or "key performance indicators" may depend on it.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

[PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Bartlomiej Zolnierkiewicz]

[ this time with full Cc: & context preserved ]

Hi,

On Wednesday, October 28, 2015 08:24:46 AM Lee Jones wrote:
> On Tue, 2015-10-27 at 18:15 +0000, Lee Jones wrote:
> > On Tue, 27 Oct 2015, Sebastian Reichel wrote:
> > > On Tue, Oct 27, 2015 at 03:42:37PM +0000, Lee Jones wrote:
> > > > Since eafbaac ("MAINTAINERS: Add "R:" designated-reviewers tag") we
> > > > have been able to tag specific people as Reviewers.  These are key
> > > > individuals who are tasked with or volunteer to review code submitted
> > > > to a subsystem or specific file.  However, according to MAINTAINERS
> > > > we have 1046 Maintainers and only a mere 22 Reviewers.  I believe
> > > > these numbers to be incorrect, as many of these Maintainers are in
> > > > fact Reviewers.
> 
> Most entries in MAINTAINERS seem to be vanity entries than actual
> active participants.  A person typically writes a driver, adds a
> MAINTAINER entry, then forgets about it and/or the hardware becomes
> outdated.
> 
> This I agree with.
> 
> On Wed, 28 Oct 2015, Krzysztof Kozlowski wrote:
> > 2015-10-28 3:44 GMT+09:00 Joe Perches <joe@perches.com>:
> > > On Tue, 2015-10-27 at 18:15 +0000, Lee Jones wrote:
> > > > On Tue, 27 Oct 2015, Sebastian Reichel wrote:> >
> > > > > I think you should CC the people, which are changed from "M:" to
> > > > > "R:", though.
> > > >
> > > > Yes, makes sense.
> > > >
> > > > I'd like to collect some Maintainer Acks first though.
> > >
> > > I think people from organizations like Samsung are actual
> > > maintainers not reviewers.
> 
> So this all hinges on how we are describing Maintainers and Reviewers.
> 
> My personal definition (until convinced otherwise) is that Reviewers
> care about their particular subsystem and/or files.  They conduct code
> reviews to ensure nothing gets broken and the code base stays in best
> possible state of worthiness.  On the other hand Maintainers usually
> conduct themselves as Reviewers but also have 'maintainership' duties
> as well; such as applying patches, *maintaining*, testing, rebasing,
> etc, an upstream branch and ultimately sending pull-requests to higher
> level Maintainers i.e. Linus.  Maintainers also have the ultimate say
> (unless over-ruled by Linus etc) over what gets applied.
> 
> > > Their drivers are not thrown over a wall and forgotten.
> > 
> > At least for Samsung Multifunction PMIC drivers (and some of Maxim
> > MUICs and PMICs) these are actively used by us in existing and new
> > products. They are also continuously extended and actually maintained.
> > This means that it is not only about review of new patches but also
> > about caring that nothing will become broken.
> 
> Exactly.  This what I expect of any good code Reviewer.
> 
> > I would prefer to leave the "SAMSUNG MULTIFUNCTION PMIC DEVICE
> > DRIVERS" entry as is - maintainers.
> 
> But you aren't maintaining the driver i.e. you don't collect patches
> and *maintain* them on an upstream branch.  Granted some of you guys
> are doing a great job of maintaining branches on your downstream or
> BSP kernels, but conduct a Reviewer type role for upstream.
> 
> You guys are pushing back like this is some kind of demotion.  That's
> not the case at all.  All it does is better describe the (very worthy)
> function you *actually* provide.

It is actually a demotion from my POV:

* "Reviewer" doesn't accurately describe the job of doing all the needed
  testing, bug-fixing and additional contributions that is often done by
  people without their own branches.

* You don't know internal policies of all companies involved in Linux
  Kernel development.  "Maintainer" is a well known term and sometimes
  person's job status or "key performance indicators" may depend on it.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Lee Jones]

On Wed, 28 Oct 2015, Bartlomiej Zolnierkiewicz wrote:
> On Wednesday, October 28, 2015 08:24:46 AM Lee Jones wrote:
> > On Tue, 2015-10-27 at 18:15 +0000, Lee Jones wrote:
> > > On Tue, 27 Oct 2015, Sebastian Reichel wrote:
> > > > On Tue, Oct 27, 2015 at 03:42:37PM +0000, Lee Jones wrote:
> > > > > Since eafbaac ("MAINTAINERS: Add "R:" designated-reviewers tag") we
> > > > > have been able to tag specific people as Reviewers.  These are key
> > > > > individuals who are tasked with or volunteer to review code submitted
> > > > > to a subsystem or specific file.  However, according to MAINTAINERS
> > > > > we have 1046 Maintainers and only a mere 22 Reviewers.  I believe
> > > > > these numbers to be incorrect, as many of these Maintainers are in
> > > > > fact Reviewers.
> > 
> > Most entries in MAINTAINERS seem to be vanity entries than actual
> > active participants.  A person typically writes a driver, adds a
> > MAINTAINER entry, then forgets about it and/or the hardware becomes
> > outdated.
> > 
> > This I agree with.
> > 
> > On Wed, 28 Oct 2015, Krzysztof Kozlowski wrote:
> > > 2015-10-28 3:44 GMT+09:00 Joe Perches <joe@perches.com>:
> > > > On Tue, 2015-10-27 at 18:15 +0000, Lee Jones wrote:
> > > > > On Tue, 27 Oct 2015, Sebastian Reichel wrote:> >
> > > > > > I think you should CC the people, which are changed from "M:" to
> > > > > > "R:", though.
> > > > >
> > > > > Yes, makes sense.
> > > > >
> > > > > I'd like to collect some Maintainer Acks first though.
> > > >
> > > > I think people from organizations like Samsung are actual
> > > > maintainers not reviewers.
> > 
> > So this all hinges on how we are describing Maintainers and Reviewers.
> > 
> > My personal definition (until convinced otherwise) is that Reviewers
> > care about their particular subsystem and/or files.  They conduct code
> > reviews to ensure nothing gets broken and the code base stays in best
> > possible state of worthiness.  On the other hand Maintainers usually
> > conduct themselves as Reviewers but also have 'maintainership' duties
> > as well; such as applying patches, *maintaining*, testing, rebasing,
> > etc, an upstream branch and ultimately sending pull-requests to higher
> > level Maintainers i.e. Linus.  Maintainers also have the ultimate say
> > (unless over-ruled by Linus etc) over what gets applied.
> > 
> > > > Their drivers are not thrown over a wall and forgotten.
> > > 
> > > At least for Samsung Multifunction PMIC drivers (and some of Maxim
> > > MUICs and PMICs) these are actively used by us in existing and new
> > > products. They are also continuously extended and actually maintained.
> > > This means that it is not only about review of new patches but also
> > > about caring that nothing will become broken.
> > 
> > Exactly.  This what I expect of any good code Reviewer.
> > 
> > > I would prefer to leave the "SAMSUNG MULTIFUNCTION PMIC DEVICE
> > > DRIVERS" entry as is - maintainers.
> > 
> > But you aren't maintaining the driver i.e. you don't collect patches
> > and *maintain* them on an upstream branch.  Granted some of you guys
> > are doing a great job of maintaining branches on your downstream or
> > BSP kernels, but conduct a Reviewer type role for upstream.
> > 
> > You guys are pushing back like this is some kind of demotion.  That's
> > not the case at all.  All it does is better describe the (very worthy)
> > function you *actually* provide.
> 
> It is actually a demotion from my POV:
> 
> * "Reviewer" doesn't accurately describe the job of doing all the needed
>   testing, bug-fixing and additional contributions that is often done by
>   people without their own branches.
> 
> * You don't know internal policies of all companies involved in Linux
>   Kernel development.  "Maintainer" is a well known term and sometimes
>   person's job status or "key performance indicators" may depend on it.

I'm going to drop the patch I think.

Despite it being the correct thing to do from a logical perspective, I
see it having too many personal, emotional and social issues/
repercussions.

Let's let sleeping dogs lie.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Lee Jones]

On Wed, 28 Oct 2015, Bartlomiej Zolnierkiewicz wrote:
> On Wednesday, October 28, 2015 08:24:46 AM Lee Jones wrote:
> > On Tue, 2015-10-27 at 18:15 +0000, Lee Jones wrote:
> > > On Tue, 27 Oct 2015, Sebastian Reichel wrote:
> > > > On Tue, Oct 27, 2015 at 03:42:37PM +0000, Lee Jones wrote:
> > > > > Since eafbaac ("MAINTAINERS: Add "R:" designated-reviewers tag") we
> > > > > have been able to tag specific people as Reviewers.  These are key
> > > > > individuals who are tasked with or volunteer to review code submitted
> > > > > to a subsystem or specific file.  However, according to MAINTAINERS
> > > > > we have 1046 Maintainers and only a mere 22 Reviewers.  I believe
> > > > > these numbers to be incorrect, as many of these Maintainers are in
> > > > > fact Reviewers.
> > 
> > Most entries in MAINTAINERS seem to be vanity entries than actual
> > active participants.  A person typically writes a driver, adds a
> > MAINTAINER entry, then forgets about it and/or the hardware becomes
> > outdated.
> > 
> > This I agree with.
> > 
> > On Wed, 28 Oct 2015, Krzysztof Kozlowski wrote:
> > > 2015-10-28 3:44 GMT+09:00 Joe Perches <joe@perches.com>:
> > > > On Tue, 2015-10-27 at 18:15 +0000, Lee Jones wrote:
> > > > > On Tue, 27 Oct 2015, Sebastian Reichel wrote:> >
> > > > > > I think you should CC the people, which are changed from "M:" to
> > > > > > "R:", though.
> > > > >
> > > > > Yes, makes sense.
> > > > >
> > > > > I'd like to collect some Maintainer Acks first though.
> > > >
> > > > I think people from organizations like Samsung are actual
> > > > maintainers not reviewers.
> > 
> > So this all hinges on how we are describing Maintainers and Reviewers.
> > 
> > My personal definition (until convinced otherwise) is that Reviewers
> > care about their particular subsystem and/or files.  They conduct code
> > reviews to ensure nothing gets broken and the code base stays in best
> > possible state of worthiness.  On the other hand Maintainers usually
> > conduct themselves as Reviewers but also have 'maintainership' duties
> > as well; such as applying patches, *maintaining*, testing, rebasing,
> > etc, an upstream branch and ultimately sending pull-requests to higher
> > level Maintainers i.e. Linus.  Maintainers also have the ultimate say
> > (unless over-ruled by Linus etc) over what gets applied.
> > 
> > > > Their drivers are not thrown over a wall and forgotten.
> > > 
> > > At least for Samsung Multifunction PMIC drivers (and some of Maxim
> > > MUICs and PMICs) these are actively used by us in existing and new
> > > products. They are also continuously extended and actually maintained.
> > > This means that it is not only about review of new patches but also
> > > about caring that nothing will become broken.
> > 
> > Exactly.  This what I expect of any good code Reviewer.
> > 
> > > I would prefer to leave the "SAMSUNG MULTIFUNCTION PMIC DEVICE
> > > DRIVERS" entry as is - maintainers.
> > 
> > But you aren't maintaining the driver i.e. you don't collect patches
> > and *maintain* them on an upstream branch.  Granted some of you guys
> > are doing a great job of maintaining branches on your downstream or
> > BSP kernels, but conduct a Reviewer type role for upstream.
> > 
> > You guys are pushing back like this is some kind of demotion.  That's
> > not the case at all.  All it does is better describe the (very worthy)
> > function you *actually* provide.
> 
> It is actually a demotion from my POV:
> 
> * "Reviewer" doesn't accurately describe the job of doing all the needed
>   testing, bug-fixing and additional contributions that is often done by
>   people without their own branches.
> 
> * You don't know internal policies of all companies involved in Linux
>   Kernel development.  "Maintainer" is a well known term and sometimes
>   person's job status or "key performance indicators" may depend on it.

I'm going to drop the patch I think.

Despite it being the correct thing to do from a logical perspective, I
see it having too many personal, emotional and social issues/
repercussions.

Let's let sleeping dogs lie.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH] get_maintainer: Add subsystem to reviewer output

[Joe Perches]

Reviewer output currently does not include the subsystem
that matched.  Add it.

Miscellanea:

o Add a get_subsystem_name routine to centralize this

Signed-off-by: Joe Perches <joe@perches.com>
---
 scripts/get_maintainer.pl | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index 98bae86..d641541 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -970,20 +970,29 @@ sub find_ending_index {
     return $index;
 }
 
-sub get_maintainer_role {
+sub get_subsystem_name {
     my ($index) = @_;
 
-    my $i;
     my $start = find_starting_index($index);
-    my $end = find_ending_index($index);
 
-    my $role = "unknown";
     my $subsystem = $typevalue[$start];
     if ($output_section_maxlen && length($subsystem) > $output_section_maxlen) {
 	$subsystem = substr($subsystem, 0, $output_section_maxlen - 3);
 	$subsystem =~ s/\s*$//;
 	$subsystem = $subsystem . "...";
     }
+    return $subsystem;
+}
+
+sub get_maintainer_role {
+    my ($index) = @_;
+
+    my $i;
+    my $start = find_starting_index($index);
+    my $end = find_ending_index($index);
+
+    my $role = "unknown";
+    my $subsystem = get_subsystem_name($index);
 
     for ($i = $start + 1; $i < $end; $i++) {
 	my $tv = $typevalue[$i];
@@ -1017,16 +1026,7 @@ sub get_maintainer_role {
 sub get_list_role {
     my ($index) = @_;
 
-    my $i;
-    my $start = find_starting_index($index);
-    my $end = find_ending_index($index);
-
-    my $subsystem = $typevalue[$start];
-    if ($output_section_maxlen && length($subsystem) > $output_section_maxlen) {
-	$subsystem = substr($subsystem, 0, $output_section_maxlen - 3);
-	$subsystem =~ s/\s*$//;
-	$subsystem = $subsystem . "...";
-    }
+    my $subsystem = get_subsystem_name($index);
 
     if ($subsystem eq "THE REST") {
 	$subsystem = "";
@@ -1114,7 +1114,8 @@ sub add_categories {
 		    }
 		}
 		if ($email_reviewer) {
-		    push_email_addresses($pvalue, 'reviewer');
+		    my $subsystem = get_subsystem_name($i);
+		    push_email_addresses($pvalue, "reviewer:$subsystem");
 		}
 	    } elsif ($ptype eq "T") {
 		push(@scm, $pvalue);

[PATCH] get_maintainer: Add subsystem to reviewer output

[Joe Perches]

Reviewer output currently does not include the subsystem
that matched.  Add it.

Miscellanea:

o Add a get_subsystem_name routine to centralize this

Signed-off-by: Joe Perches <joe@perches.com>
---
 scripts/get_maintainer.pl | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index 98bae86..d641541 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -970,20 +970,29 @@ sub find_ending_index {
     return $index;
 }
 
-sub get_maintainer_role {
+sub get_subsystem_name {
     my ($index) = @_;
 
-    my $i;
     my $start = find_starting_index($index);
-    my $end = find_ending_index($index);
 
-    my $role = "unknown";
     my $subsystem = $typevalue[$start];
     if ($output_section_maxlen && length($subsystem) > $output_section_maxlen) {
 	$subsystem = substr($subsystem, 0, $output_section_maxlen - 3);
 	$subsystem =~ s/\s*$//;
 	$subsystem = $subsystem . "...";
     }
+    return $subsystem;
+}
+
+sub get_maintainer_role {
+    my ($index) = @_;
+
+    my $i;
+    my $start = find_starting_index($index);
+    my $end = find_ending_index($index);
+
+    my $role = "unknown";
+    my $subsystem = get_subsystem_name($index);
 
     for ($i = $start + 1; $i < $end; $i++) {
 	my $tv = $typevalue[$i];
@@ -1017,16 +1026,7 @@ sub get_maintainer_role {
 sub get_list_role {
     my ($index) = @_;
 
-    my $i;
-    my $start = find_starting_index($index);
-    my $end = find_ending_index($index);
-
-    my $subsystem = $typevalue[$start];
-    if ($output_section_maxlen && length($subsystem) > $output_section_maxlen) {
-	$subsystem = substr($subsystem, 0, $output_section_maxlen - 3);
-	$subsystem =~ s/\s*$//;
-	$subsystem = $subsystem . "...";
-    }
+    my $subsystem = get_subsystem_name($index);
 
     if ($subsystem eq "THE REST") {
 	$subsystem = "";
@@ -1114,7 +1114,8 @@ sub add_categories {
 		    }
 		}
 		if ($email_reviewer) {
-		    push_email_addresses($pvalue, 'reviewer');
+		    my $subsystem = get_subsystem_name($i);
+		    push_email_addresses($pvalue, "reviewer:$subsystem");
 		}
 	    } elsif ($ptype eq "T") {
 		push(@scm, $pvalue);

Re: [PATCH] get_maintainer: Add subsystem to reviewer output

[Lee Jones]

On Wed, 28 Oct 2015, Joe Perches wrote:

> Reviewer output currently does not include the subsystem
> that matched.  Add it.
> 
> Miscellanea:
> 
> o Add a get_subsystem_name routine to centralize this
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  scripts/get_maintainer.pl | 31 ++++++++++++++++---------------
>  1 file changed, 16 insertions(+), 15 deletions(-)

It looks like I'm going to have to drop the patch where we actually
start using the R: tag properly due to some social, emotional issues.

Despite that and not knowing Perl, I believe get_maintainer.pl should
be printing out the subsystem next to 'reviewer' as it does for
'maintainer'.

So with that in mind:
  Acked-by: Lee Jones <lee.jones@linaro.org>

> diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
> index 98bae86..d641541 100755
> --- a/scripts/get_maintainer.pl
> +++ b/scripts/get_maintainer.pl
> @@ -970,20 +970,29 @@ sub find_ending_index {
>      return $index;
>  }
>  
> -sub get_maintainer_role {
> +sub get_subsystem_name {
>      my ($index) = @_;
>  
> -    my $i;
>      my $start = find_starting_index($index);
> -    my $end = find_ending_index($index);
>  
> -    my $role = "unknown";
>      my $subsystem = $typevalue[$start];
>      if ($output_section_maxlen && length($subsystem) > $output_section_maxlen) {
>  	$subsystem = substr($subsystem, 0, $output_section_maxlen - 3);
>  	$subsystem =~ s/\s*$//;
>  	$subsystem = $subsystem . "...";
>      }
> +    return $subsystem;
> +}
> +
> +sub get_maintainer_role {
> +    my ($index) = @_;
> +
> +    my $i;
> +    my $start = find_starting_index($index);
> +    my $end = find_ending_index($index);
> +
> +    my $role = "unknown";
> +    my $subsystem = get_subsystem_name($index);
>  
>      for ($i = $start + 1; $i < $end; $i++) {
>  	my $tv = $typevalue[$i];
> @@ -1017,16 +1026,7 @@ sub get_maintainer_role {
>  sub get_list_role {
>      my ($index) = @_;
>  
> -    my $i;
> -    my $start = find_starting_index($index);
> -    my $end = find_ending_index($index);
> -
> -    my $subsystem = $typevalue[$start];
> -    if ($output_section_maxlen && length($subsystem) > $output_section_maxlen) {
> -	$subsystem = substr($subsystem, 0, $output_section_maxlen - 3);
> -	$subsystem =~ s/\s*$//;
> -	$subsystem = $subsystem . "...";
> -    }
> +    my $subsystem = get_subsystem_name($index);
>  
>      if ($subsystem eq "THE REST") {
>  	$subsystem = "";
> @@ -1114,7 +1114,8 @@ sub add_categories {
>  		    }
>  		}
>  		if ($email_reviewer) {
> -		    push_email_addresses($pvalue, 'reviewer');
> +		    my $subsystem = get_subsystem_name($i);
> +		    push_email_addresses($pvalue, "reviewer:$subsystem");
>  		}
>  	    } elsif ($ptype eq "T") {
>  		push(@scm, $pvalue);
> 
> 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH] get_maintainer: Add subsystem to reviewer output

[Lee Jones]

On Wed, 28 Oct 2015, Joe Perches wrote:

> Reviewer output currently does not include the subsystem
> that matched.  Add it.
> 
> Miscellanea:
> 
> o Add a get_subsystem_name routine to centralize this
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  scripts/get_maintainer.pl | 31 ++++++++++++++++---------------
>  1 file changed, 16 insertions(+), 15 deletions(-)

It looks like I'm going to have to drop the patch where we actually
start using the R: tag properly due to some social, emotional issues.

Despite that and not knowing Perl, I believe get_maintainer.pl should
be printing out the subsystem next to 'reviewer' as it does for
'maintainer'.

So with that in mind:
  Acked-by: Lee Jones <lee.jones@linaro.org>

> diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
> index 98bae86..d641541 100755
> --- a/scripts/get_maintainer.pl
> +++ b/scripts/get_maintainer.pl
> @@ -970,20 +970,29 @@ sub find_ending_index {
>      return $index;
>  }
>  
> -sub get_maintainer_role {
> +sub get_subsystem_name {
>      my ($index) = @_;
>  
> -    my $i;
>      my $start = find_starting_index($index);
> -    my $end = find_ending_index($index);
>  
> -    my $role = "unknown";
>      my $subsystem = $typevalue[$start];
>      if ($output_section_maxlen && length($subsystem) > $output_section_maxlen) {
>  	$subsystem = substr($subsystem, 0, $output_section_maxlen - 3);
>  	$subsystem =~ s/\s*$//;
>  	$subsystem = $subsystem . "...";
>      }
> +    return $subsystem;
> +}
> +
> +sub get_maintainer_role {
> +    my ($index) = @_;
> +
> +    my $i;
> +    my $start = find_starting_index($index);
> +    my $end = find_ending_index($index);
> +
> +    my $role = "unknown";
> +    my $subsystem = get_subsystem_name($index);
>  
>      for ($i = $start + 1; $i < $end; $i++) {
>  	my $tv = $typevalue[$i];
> @@ -1017,16 +1026,7 @@ sub get_maintainer_role {
>  sub get_list_role {
>      my ($index) = @_;
>  
> -    my $i;
> -    my $start = find_starting_index($index);
> -    my $end = find_ending_index($index);
> -
> -    my $subsystem = $typevalue[$start];
> -    if ($output_section_maxlen && length($subsystem) > $output_section_maxlen) {
> -	$subsystem = substr($subsystem, 0, $output_section_maxlen - 3);
> -	$subsystem =~ s/\s*$//;
> -	$subsystem = $subsystem . "...";
> -    }
> +    my $subsystem = get_subsystem_name($index);
>  
>      if ($subsystem eq "THE REST") {
>  	$subsystem = "";
> @@ -1114,7 +1114,8 @@ sub add_categories {
>  		    }
>  		}
>  		if ($email_reviewer) {
> -		    push_email_addresses($pvalue, 'reviewer');
> +		    my $subsystem = get_subsystem_name($i);
> +		    push_email_addresses($pvalue, "reviewer:$subsystem");
>  		}
>  	    } elsif ($ptype eq "T") {
>  		push(@scm, $pvalue);
> 
> 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] get_maintainer: Add subsystem to reviewer output

[Joe Perches]

On Wed, 2015-10-28 at 17:01 +0000, Lee Jones wrote:
> On Wed, 28 Oct 2015, Joe Perches wrote:
> 
> > Reviewer output currently does not include the subsystem
> > that matched.  Add it.
> > 
> > Miscellanea:
> > 
> > o Add a get_subsystem_name routine to centralize this
> > 
> > Signed-off-by: Joe Perches <joe@perches.com>
> > ---
> >  scripts/get_maintainer.pl | 31 ++++++++++++++++---------------
> >  1 file changed, 16 insertions(+), 15 deletions(-)
> 
> It looks like I'm going to have to drop the patch where we actually
> start using the R: tag properly due to some social, emotional issues.

Not to mention logical.

> Despite that and not knowing Perl, I believe get_maintainer.pl should
> be printing out the subsystem next to 'reviewer' as it does for
> 'maintainer'.
> 
> So with that in mind:
>   Acked-by: Lee Jones <lee.jones@linaro.org>

It'd be better if you could add a "tested-by:" instead.

[PATCH] get_maintainer: Add subsystem to reviewer output

[Joe Perches]

On Wed, 2015-10-28 at 17:01 +0000, Lee Jones wrote:
> On Wed, 28 Oct 2015, Joe Perches wrote:
> 
> > Reviewer output currently does not include the subsystem
> > that matched.  Add it.
> > 
> > Miscellanea:
> > 
> > o Add a get_subsystem_name routine to centralize this
> > 
> > Signed-off-by: Joe Perches <joe@perches.com>
> > ---
> >  scripts/get_maintainer.pl | 31 ++++++++++++++++---------------
> >  1 file changed, 16 insertions(+), 15 deletions(-)
> 
> It looks like I'm going to have to drop the patch where we actually
> start using the R: tag properly due to some social, emotional issues.

Not to mention logical.

> Despite that and not knowing Perl, I believe get_maintainer.pl should
> be printing out the subsystem next to 'reviewer' as it does for
> 'maintainer'.
> 
> So with that in mind:
>   Acked-by: Lee Jones <lee.jones@linaro.org>

It'd be better if you could add a "tested-by:" instead.

Re: [PATCH] get_maintainer: Add subsystem to reviewer output

[Joe Perches]

On Wed, 2015-10-28 at 17:01 +0000, Lee Jones wrote:
> It looks like I'm going to have to drop the patch where we actually
> start using the R: tag properly due to some social, emotional issues.

btw: it seems to me the thing you're looking for is the tree
to which any particular patch could or would be applied.

That can be got by using the --scm option:

	./scripts/get_maintainer.pl --scm <patch>

Only ~400 of ~1445 subsystem sections have a "T:" tree entry
so pairing the maintainer and the tree generally tells you
who is responsible for upstreaming any patch.

[PATCH] get_maintainer: Add subsystem to reviewer output

[Joe Perches]

On Wed, 2015-10-28 at 17:01 +0000, Lee Jones wrote:
> It looks like I'm going to have to drop the patch where we actually
> start using the R: tag properly due to some social, emotional issues.

btw: it seems to me the thing you're looking for is the tree
to which any particular patch could or would be applied.

That can be got by using the --scm option:

	./scripts/get_maintainer.pl --scm <patch>

Only ~400 of ~1445 subsystem sections have a "T:" tree entry
so pairing the maintainer and the tree generally tells you
who is responsible for upstreaming any patch.

Re: [PATCH] get_maintainer: Add subsystem to reviewer output

[Lee Jones]

On Wed, 28 Oct 2015, Joe Perches wrote:

> On Wed, 2015-10-28 at 17:01 +0000, Lee Jones wrote:
> > On Wed, 28 Oct 2015, Joe Perches wrote:
> > 
> > > Reviewer output currently does not include the subsystem
> > > that matched.  Add it.
> > > 
> > > Miscellanea:
> > > 
> > > o Add a get_subsystem_name routine to centralize this
> > > 
> > > Signed-off-by: Joe Perches <joe@perches.com>
> > > ---
> > >  scripts/get_maintainer.pl | 31 ++++++++++++++++---------------
> > >  1 file changed, 16 insertions(+), 15 deletions(-)
> > 
> > It looks like I'm going to have to drop the patch where we actually
> > start using the R: tag properly due to some social, emotional issues.
> 
> Not to mention logical.

Hmm... we continue to disagree on that one.

> > Despite that and not knowing Perl, I believe get_maintainer.pl should
> > be printing out the subsystem next to 'reviewer' as it does for
> > 'maintainer'.
> > 
> > So with that in mind:
> >   Acked-by: Lee Jones <lee.jones@linaro.org>
> 
> It'd be better if you could add a "tested-by:" instead.

Patch works as expected:
  Tested-by: Lee Jones <lee.jones@linaro.org>

Feel free to add my Suggested-by too, based on my belief that this
patch was born out of this comment:

      "I'm sure we can make the output even more similar by listing
      the MAINTAINERS tag after "reviewer" too."

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH] get_maintainer: Add subsystem to reviewer output

[Lee Jones]

On Wed, 28 Oct 2015, Joe Perches wrote:

> On Wed, 2015-10-28 at 17:01 +0000, Lee Jones wrote:
> > On Wed, 28 Oct 2015, Joe Perches wrote:
> > 
> > > Reviewer output currently does not include the subsystem
> > > that matched.  Add it.
> > > 
> > > Miscellanea:
> > > 
> > > o Add a get_subsystem_name routine to centralize this
> > > 
> > > Signed-off-by: Joe Perches <joe@perches.com>
> > > ---
> > >  scripts/get_maintainer.pl | 31 ++++++++++++++++---------------
> > >  1 file changed, 16 insertions(+), 15 deletions(-)
> > 
> > It looks like I'm going to have to drop the patch where we actually
> > start using the R: tag properly due to some social, emotional issues.
> 
> Not to mention logical.

Hmm... we continue to disagree on that one.

> > Despite that and not knowing Perl, I believe get_maintainer.pl should
> > be printing out the subsystem next to 'reviewer' as it does for
> > 'maintainer'.
> > 
> > So with that in mind:
> >   Acked-by: Lee Jones <lee.jones@linaro.org>
> 
> It'd be better if you could add a "tested-by:" instead.

Patch works as expected:
  Tested-by: Lee Jones <lee.jones@linaro.org>

Feel free to add my Suggested-by too, based on my belief that this
patch was born out of this comment:

      "I'm sure we can make the output even more similar by listing
      the MAINTAINERS tag after "reviewer" too."

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] get_maintainer: Add subsystem to reviewer output

[Joe Perches]

On Wed, 2015-10-28 at 17:22 +0000, Lee Jones wrote:
> On Wed, 28 Oct 2015, Joe Perches wrote:
> > >   Acked-by: Lee Jones <lee.jones@linaro.org>
> > It'd be better if you could add a "tested-by:" instead.
> 
> Patch works as expected:
>   Tested-by: Lee Jones <lee.jones@linaro.org>
[]
>       "I'm sure we can make the output even more similar by listing
>       the MAINTAINERS tag after "reviewer" too."

I guess we'll disagree on that one too.

[PATCH] get_maintainer: Add subsystem to reviewer output

[Joe Perches]

On Wed, 2015-10-28 at 17:22 +0000, Lee Jones wrote:
> On Wed, 28 Oct 2015, Joe Perches wrote:
> > >   Acked-by: Lee Jones <lee.jones@linaro.org>
> > It'd be better if you could add a "tested-by:" instead.
> 
> Patch works as expected:
>   Tested-by: Lee Jones <lee.jones@linaro.org>
[]
>       "I'm sure we can make the output even more similar by listing
>       the MAINTAINERS tag after "reviewer" too."

I guess we'll disagree on that one too.

Re: [PATCH] get_maintainer: Add subsystem to reviewer output

[Lee Jones]

On Wed, 28 Oct 2015, Joe Perches wrote:

> On Wed, 2015-10-28 at 17:22 +0000, Lee Jones wrote:
> > On Wed, 28 Oct 2015, Joe Perches wrote:
> > > >   Acked-by: Lee Jones <lee.jones@linaro.org>
> > > It'd be better if you could add a "tested-by:" instead.
> > 
> > Patch works as expected:
> >   Tested-by: Lee Jones <lee.jones@linaro.org>
> []
> >       "I'm sure we can make the output even more similar by listing
> >       the MAINTAINERS tag after "reviewer" too."
> 
> I guess we'll disagree on that one too.

You just happened to think of it a few hours after I mentioned it.

Remarkable. ;)

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH] get_maintainer: Add subsystem to reviewer output

[Lee Jones]

On Wed, 28 Oct 2015, Joe Perches wrote:

> On Wed, 2015-10-28 at 17:22 +0000, Lee Jones wrote:
> > On Wed, 28 Oct 2015, Joe Perches wrote:
> > > >   Acked-by: Lee Jones <lee.jones@linaro.org>
> > > It'd be better if you could add a "tested-by:" instead.
> > 
> > Patch works as expected:
> >   Tested-by: Lee Jones <lee.jones@linaro.org>
> []
> >       "I'm sure we can make the output even more similar by listing
> >       the MAINTAINERS tag after "reviewer" too."
> 
> I guess we'll disagree on that one too.

You just happened to think of it a few hours after I mentioned it.

Remarkable. ;)

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] get_maintainer: Add subsystem to reviewer output

[Joe Perches]

On Wed, 2015-10-28 at 17:49 +0000, Lee Jones wrote:
> On Wed, 28 Oct 2015, Joe Perches wrote:
> > On Wed, 2015-10-28 at 17:22 +0000, Lee Jones wrote:
> > > On Wed, 28 Oct 2015, Joe Perches wrote:
> > > > >   Acked-by: Lee Jones <lee.jones@linaro.org>
> > > > It'd be better if you could add a "tested-by:" instead.
> > > 
> > > Patch works as expected:
> > >   Tested-by: Lee Jones <lee.jones@linaro.org>
> > []
> > >       "I'm sure we can make the output even more similar by listing
> > >       the MAINTAINERS tag after "reviewer" too."
> > 
> > I guess we'll disagree on that one too.
> 
> You just happened to think of it a few hours after I mentioned it.

Nope, it seems a maintainers tag is what Krzysztof was suggesting
with his "supported reviewer" bit.

I added the subsystem name.

An issue for that will be how multiple subsystem section matching
affects the output for reviewers who are also maintainers.

[PATCH] get_maintainer: Add subsystem to reviewer output

[Joe Perches]

On Wed, 2015-10-28 at 17:49 +0000, Lee Jones wrote:
> On Wed, 28 Oct 2015, Joe Perches wrote:
> > On Wed, 2015-10-28 at 17:22 +0000, Lee Jones wrote:
> > > On Wed, 28 Oct 2015, Joe Perches wrote:
> > > > >   Acked-by: Lee Jones <lee.jones@linaro.org>
> > > > It'd be better if you could add a "tested-by:" instead.
> > > 
> > > Patch works as expected:
> > >   Tested-by: Lee Jones <lee.jones@linaro.org>
> > []
> > >       "I'm sure we can make the output even more similar by listing
> > >       the MAINTAINERS tag after "reviewer" too."
> > 
> > I guess we'll disagree on that one too.
> 
> You just happened to think of it a few hours after I mentioned it.

Nope, it seems a maintainers tag is what Krzysztof was suggesting
with his "supported reviewer" bit.

I added the subsystem name.

An issue for that will be how multiple subsystem section matching
affects the output for reviewers who are also maintainers.

Re: [PATCH] get_maintainer: Add subsystem to reviewer output

[Krzysztof Kozlowski]

On 29.10.2015 01:41, Joe Perches wrote:
> Reviewer output currently does not include the subsystem
> that matched.  Add it.
> 
> Miscellanea:
> 
> o Add a get_subsystem_name routine to centralize this
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  scripts/get_maintainer.pl | 31 ++++++++++++++++---------------
>  1 file changed, 16 insertions(+), 15 deletions(-)
> 

Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Best regards,
Krzysztof

[PATCH] get_maintainer: Add subsystem to reviewer output

[Krzysztof Kozlowski]

On 29.10.2015 01:41, Joe Perches wrote:
> Reviewer output currently does not include the subsystem
> that matched.  Add it.
> 
> Miscellanea:
> 
> o Add a get_subsystem_name routine to centralize this
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  scripts/get_maintainer.pl | 31 ++++++++++++++++---------------
>  1 file changed, 16 insertions(+), 15 deletions(-)
> 

Tested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Best regards,
Krzysztof

Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Krzysztof Kozlowski]

On 28.10.2015 23:38, Lee Jones wrote:
> On Wed, 28 Oct 2015, Javier Martinez Canillas wrote:
>> They are not maintainers according to your definition of maintainer
>> that doesn't seem what most people agree with.
> 
> "most people" so far are 3 people that I assume still want to be
> Maintainers despite not actually conducting Maintainer duties, but
> are rather Reviewers.  I also have 2 Acks for this patch, so thus far
> that's 3 that agree and 3 that do not.  Unsurprisingly the ones that
> agree are Maintainers and the ones who are not are (by my definition)
> Reviewers -- go figure.
> 

I am not sure on which side you put me finally. :)
If there is a consensus among some more experienced developers that
maintainer means branch and patches maintaining, then I won't see any
problem with the patch nor with switching Samsung PMIC entries to review.

In that case, that would be:
Acked-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Best regards,
Krzysztof

[PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Krzysztof Kozlowski]

On 28.10.2015 23:38, Lee Jones wrote:
> On Wed, 28 Oct 2015, Javier Martinez Canillas wrote:
>> They are not maintainers according to your definition of maintainer
>> that doesn't seem what most people agree with.
> 
> "most people" so far are 3 people that I assume still want to be
> Maintainers despite not actually conducting Maintainer duties, but
> are rather Reviewers.  I also have 2 Acks for this patch, so thus far
> that's 3 that agree and 3 that do not.  Unsurprisingly the ones that
> agree are Maintainers and the ones who are not are (by my definition)
> Reviewers -- go figure.
> 

I am not sure on which side you put me finally. :)
If there is a consensus among some more experienced developers that
maintainer means branch and patches maintaining, then I won't see any
problem with the patch nor with switching Samsung PMIC entries to review.

In that case, that would be:
Acked-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Best regards,
Krzysztof

Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Javier Martinez Canillas]

Hello Lee,

On Thu, Oct 29, 2015 at 12:56 AM, Krzysztof Kozlowski
<k.kozlowski@samsung.com> wrote:
> On 28.10.2015 23:38, Lee Jones wrote:
>> On Wed, 28 Oct 2015, Javier Martinez Canillas wrote:
>>> They are not maintainers according to your definition of maintainer
>>> that doesn't seem what most people agree with.
>>
>> "most people" so far are 3 people that I assume still want to be
>> Maintainers despite not actually conducting Maintainer duties, but
>> are rather Reviewers.  I also have 2 Acks for this patch, so thus far
>> that's 3 that agree and 3 that do not.  Unsurprisingly the ones that
>> agree are Maintainers and the ones who are not are (by my definition)
>> Reviewers -- go figure.
>>
>
> I am not sure on which side you put me finally. :)
> If there is a consensus among some more experienced developers that
> maintainer means branch and patches maintaining, then I won't see any
> problem with the patch nor with switching Samsung PMIC entries to review.
>
> In that case, that would be:
> Acked-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>

Same for me, what I don't want is to have different meanings per
subsystems of what maintainers and reviewers mean since that could
confuse developers posting patches.

But if there is a kernel wide consensus and all subsystems entries are
going to be updated to use the same semantics and list as Reviewer to
people that don't keep git trees, then I'm OK with this patch and you
can add my:

Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

> Best regards,
> Krzysztof
>

Best regards,
Javier

[PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Javier Martinez Canillas]

Hello Lee,

On Thu, Oct 29, 2015 at 12:56 AM, Krzysztof Kozlowski
<k.kozlowski@samsung.com> wrote:
> On 28.10.2015 23:38, Lee Jones wrote:
>> On Wed, 28 Oct 2015, Javier Martinez Canillas wrote:
>>> They are not maintainers according to your definition of maintainer
>>> that doesn't seem what most people agree with.
>>
>> "most people" so far are 3 people that I assume still want to be
>> Maintainers despite not actually conducting Maintainer duties, but
>> are rather Reviewers.  I also have 2 Acks for this patch, so thus far
>> that's 3 that agree and 3 that do not.  Unsurprisingly the ones that
>> agree are Maintainers and the ones who are not are (by my definition)
>> Reviewers -- go figure.
>>
>
> I am not sure on which side you put me finally. :)
> If there is a consensus among some more experienced developers that
> maintainer means branch and patches maintaining, then I won't see any
> problem with the patch nor with switching Samsung PMIC entries to review.
>
> In that case, that would be:
> Acked-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>

Same for me, what I don't want is to have different meanings per
subsystems of what maintainers and reviewers mean since that could
confuse developers posting patches.

But if there is a kernel wide consensus and all subsystems entries are
going to be updated to use the same semantics and list as Reviewer to
people that don't keep git trees, then I'm OK with this patch and you
can add my:

Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

> Best regards,
> Krzysztof
>

Best regards,
Javier

Re: [PATCH] get_maintainer: Add subsystem to reviewer output

[Lee Jones]

On Wed, 28 Oct 2015, Joe Perches wrote:
> On Wed, 2015-10-28 at 17:49 +0000, Lee Jones wrote:
> > On Wed, 28 Oct 2015, Joe Perches wrote:
> > > On Wed, 2015-10-28 at 17:22 +0000, Lee Jones wrote:
> > > > On Wed, 28 Oct 2015, Joe Perches wrote:
> > > > > >   Acked-by: Lee Jones <lee.jones@linaro.org>
> > > > > It'd be better if you could add a "tested-by:" instead.
> > > > 
> > > > Patch works as expected:
> > > >   Tested-by: Lee Jones <lee.jones@linaro.org>
> > > []
> > > >       "I'm sure we can make the output even more similar by listing
> > > >       the MAINTAINERS tag after "reviewer" too."
> > > 
> > > I guess we'll disagree on that one too.
> > 
> > You just happened to think of it a few hours after I mentioned it.
> 
> Nope, it seems a maintainers tag is what Krzysztof was suggesting
> with his "supported reviewer" bit.

Which was in reply to the bit above.

> I added the subsystem name.

(subsystem name == MAINTAINERS tag) in my comment above.

> An issue for that will be how multiple subsystem section matching
> affects the output for reviewers who are also maintainers.

I think that's okay, becuase the 'MAINTAINERS tag' will be different.

As an example:

  Lee Jones <lee.jones@linaro.org> (supporter:DRIVER SUBSYSTEM)
  Lee Jones <lee.jones@linaro.org> (reviewer:VENDOR DRIVER NAME)

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH] get_maintainer: Add subsystem to reviewer output

[Lee Jones]

On Wed, 28 Oct 2015, Joe Perches wrote:
> On Wed, 2015-10-28 at 17:49 +0000, Lee Jones wrote:
> > On Wed, 28 Oct 2015, Joe Perches wrote:
> > > On Wed, 2015-10-28 at 17:22 +0000, Lee Jones wrote:
> > > > On Wed, 28 Oct 2015, Joe Perches wrote:
> > > > > >   Acked-by: Lee Jones <lee.jones@linaro.org>
> > > > > It'd be better if you could add a "tested-by:" instead.
> > > > 
> > > > Patch works as expected:
> > > >   Tested-by: Lee Jones <lee.jones@linaro.org>
> > > []
> > > >       "I'm sure we can make the output even more similar by listing
> > > >       the MAINTAINERS tag after "reviewer" too."
> > > 
> > > I guess we'll disagree on that one too.
> > 
> > You just happened to think of it a few hours after I mentioned it.
> 
> Nope, it seems a maintainers tag is what Krzysztof was suggesting
> with his "supported reviewer" bit.

Which was in reply to the bit above.

> I added the subsystem name.

(subsystem name == MAINTAINERS tag) in my comment above.

> An issue for that will be how multiple subsystem section matching
> affects the output for reviewers who are also maintainers.

I think that's okay, becuase the 'MAINTAINERS tag' will be different.

As an example:

  Lee Jones <lee.jones@linaro.org> (supporter:DRIVER SUBSYSTEM)
  Lee Jones <lee.jones@linaro.org> (reviewer:VENDOR DRIVER NAME)

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] get_maintainer: Add subsystem to reviewer output

[Joe Perches]

On Thu, 2015-10-29 at 09:20 +0000, Lee Jones wrote:
> On Wed, 28 Oct 2015, Joe Perches wrote:

> > An issue for that will be how multiple subsystem section matching
> > affects the output for reviewers who are also maintainers.
> 
> I think that's okay, becuase the 'MAINTAINERS tag' will be different.
> 
> As an example:
> 
>   Lee Jones <lee.jones@linaro.org> (supporter:DRIVER SUBSYSTEM)
>   Lee Jones <lee.jones@linaro.org> (reviewer:VENDOR DRIVER NAME)

I'm pretty sure I know a little more about the
behavior of the get_maintainers script than you do.

[PATCH] get_maintainer: Add subsystem to reviewer output

[Joe Perches]

On Thu, 2015-10-29 at 09:20 +0000, Lee Jones wrote:
> On Wed, 28 Oct 2015, Joe Perches wrote:

> > An issue for that will be how multiple subsystem section matching
> > affects the output for reviewers who are also maintainers.
> 
> I think that's okay, becuase the 'MAINTAINERS tag' will be different.
> 
> As an example:
> 
>   Lee Jones <lee.jones@linaro.org> (supporter:DRIVER SUBSYSTEM)
>   Lee Jones <lee.jones@linaro.org> (reviewer:VENDOR DRIVER NAME)

I'm pretty sure I know a little more about the
behavior of the get_maintainers script than you do.

Re: [PATCH] get_maintainer: Add subsystem to reviewer output

[Lee Jones]

On 29 October 2015 at 14:14, Joe Perches <joe@perches.com> wrote:
> On Thu, 2015-10-29 at 09:20 +0000, Lee Jones wrote:
>> On Wed, 28 Oct 2015, Joe Perches wrote:
>
>> > An issue for that will be how multiple subsystem section matching
>> > affects the output for reviewers who are also maintainers.
>>
>> I think that's okay, becuase the 'MAINTAINERS tag' will be different.
>>
>> As an example:
>>
>>   Lee Jones <lee.jones@linaro.org> (supporter:DRIVER SUBSYSTEM)
>>   Lee Jones <lee.jones@linaro.org> (reviewer:VENDOR DRIVER NAME)
>
> I'm pretty sure I know a little more about the
> behavior of the get_maintainers script than you do.

Instead of this childishness, I'm guessing what you meant to say was;
unfortunately the current behaviour of the script is such that, if a
person is marked as a Reviewer and a Maintainer, the final one
mentioned in MAINTAINERS will take precedence over each of the others.
In other words, only one (the final) reference will be printed.

If so, thanks Joe that's informative, and yes, I can see how that
might cause issues in some cases.

-- 
Lee Jones
Linaro ST Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH] get_maintainer: Add subsystem to reviewer output

[Lee Jones]

On 29 October 2015 at 14:14, Joe Perches <joe@perches.com> wrote:
> On Thu, 2015-10-29 at 09:20 +0000, Lee Jones wrote:
>> On Wed, 28 Oct 2015, Joe Perches wrote:
>
>> > An issue for that will be how multiple subsystem section matching
>> > affects the output for reviewers who are also maintainers.
>>
>> I think that's okay, becuase the 'MAINTAINERS tag' will be different.
>>
>> As an example:
>>
>>   Lee Jones <lee.jones@linaro.org> (supporter:DRIVER SUBSYSTEM)
>>   Lee Jones <lee.jones@linaro.org> (reviewer:VENDOR DRIVER NAME)
>
> I'm pretty sure I know a little more about the
> behavior of the get_maintainers script than you do.

Instead of this childishness, I'm guessing what you meant to say was;
unfortunately the current behaviour of the script is such that, if a
person is marked as a Reviewer and a Maintainer, the final one
mentioned in MAINTAINERS will take precedence over each of the others.
In other words, only one (the final) reference will be printed.

If so, thanks Joe that's informative, and yes, I can see how that
might cause issues in some cases.

-- 
Lee Jones
Linaro ST Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Lee Jones]

On Thu, 29 Oct 2015, Krzysztof Kozlowski wrote:

> On 28.10.2015 23:38, Lee Jones wrote:
> > On Wed, 28 Oct 2015, Javier Martinez Canillas wrote:
> >> They are not maintainers according to your definition of maintainer
> >> that doesn't seem what most people agree with.
> > 
> > "most people" so far are 3 people that I assume still want to be
> > Maintainers despite not actually conducting Maintainer duties, but
> > are rather Reviewers.  I also have 2 Acks for this patch, so thus far
> > that's 3 that agree and 3 that do not.  Unsurprisingly the ones that
> > agree are Maintainers and the ones who are not are (by my definition)
> > Reviewers -- go figure.
> > 
> 
> I am not sure on which side you put me finally. :)
> If there is a consensus among some more experienced developers that
> maintainer means branch and patches maintaining, then I won't see any
> problem with the patch nor with switching Samsung PMIC entries to review.
> 
> In that case, that would be:
> Acked-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Thanks Krzysztof, but I'm going to drop this patch.

I can see it causing more issues than it'll solve.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Lee Jones]

On Thu, 29 Oct 2015, Krzysztof Kozlowski wrote:

> On 28.10.2015 23:38, Lee Jones wrote:
> > On Wed, 28 Oct 2015, Javier Martinez Canillas wrote:
> >> They are not maintainers according to your definition of maintainer
> >> that doesn't seem what most people agree with.
> > 
> > "most people" so far are 3 people that I assume still want to be
> > Maintainers despite not actually conducting Maintainer duties, but
> > are rather Reviewers.  I also have 2 Acks for this patch, so thus far
> > that's 3 that agree and 3 that do not.  Unsurprisingly the ones that
> > agree are Maintainers and the ones who are not are (by my definition)
> > Reviewers -- go figure.
> > 
> 
> I am not sure on which side you put me finally. :)
> If there is a consensus among some more experienced developers that
> maintainer means branch and patches maintaining, then I won't see any
> problem with the patch nor with switching Samsung PMIC entries to review.
> 
> In that case, that would be:
> Acked-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>

Thanks Krzysztof, but I'm going to drop this patch.

I can see it causing more issues than it'll solve.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Lee Jones]

On Thu, 29 Oct 2015, Javier Martinez Canillas wrote:

> Hello Lee,
> 
> On Thu, Oct 29, 2015 at 12:56 AM, Krzysztof Kozlowski
> <k.kozlowski@samsung.com> wrote:
> > On 28.10.2015 23:38, Lee Jones wrote:
> >> On Wed, 28 Oct 2015, Javier Martinez Canillas wrote:
> >>> They are not maintainers according to your definition of maintainer
> >>> that doesn't seem what most people agree with.
> >>
> >> "most people" so far are 3 people that I assume still want to be
> >> Maintainers despite not actually conducting Maintainer duties, but
> >> are rather Reviewers.  I also have 2 Acks for this patch, so thus far
> >> that's 3 that agree and 3 that do not.  Unsurprisingly the ones that
> >> agree are Maintainers and the ones who are not are (by my definition)
> >> Reviewers -- go figure.
> >>
> >
> > I am not sure on which side you put me finally. :)
> > If there is a consensus among some more experienced developers that
> > maintainer means branch and patches maintaining, then I won't see any
> > problem with the patch nor with switching Samsung PMIC entries to review.
> >
> > In that case, that would be:
> > Acked-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> >
> 
> Same for me, what I don't want is to have different meanings per
> subsystems of what maintainers and reviewers mean since that could
> confuse developers posting patches.
> 
> But if there is a kernel wide consensus and all subsystems entries are
> going to be updated to use the same semantics and list as Reviewer to
> people that don't keep git trees, then I'm OK with this patch and you
> can add my:
> 
> Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

Thanks Javier, but I'm going to drop this patch.

I can see it causing more issues than it'll solve.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag

[Lee Jones]

On Thu, 29 Oct 2015, Javier Martinez Canillas wrote:

> Hello Lee,
> 
> On Thu, Oct 29, 2015 at 12:56 AM, Krzysztof Kozlowski
> <k.kozlowski@samsung.com> wrote:
> > On 28.10.2015 23:38, Lee Jones wrote:
> >> On Wed, 28 Oct 2015, Javier Martinez Canillas wrote:
> >>> They are not maintainers according to your definition of maintainer
> >>> that doesn't seem what most people agree with.
> >>
> >> "most people" so far are 3 people that I assume still want to be
> >> Maintainers despite not actually conducting Maintainer duties, but
> >> are rather Reviewers.  I also have 2 Acks for this patch, so thus far
> >> that's 3 that agree and 3 that do not.  Unsurprisingly the ones that
> >> agree are Maintainers and the ones who are not are (by my definition)
> >> Reviewers -- go figure.
> >>
> >
> > I am not sure on which side you put me finally. :)
> > If there is a consensus among some more experienced developers that
> > maintainer means branch and patches maintaining, then I won't see any
> > problem with the patch nor with switching Samsung PMIC entries to review.
> >
> > In that case, that would be:
> > Acked-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> >
> 
> Same for me, what I don't want is to have different meanings per
> subsystems of what maintainers and reviewers mean since that could
> confuse developers posting patches.
> 
> But if there is a kernel wide consensus and all subsystems entries are
> going to be updated to use the same semantics and list as Reviewer to
> people that don't keep git trees, then I'm OK with this patch and you
> can add my:
> 
> Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

Thanks Javier, but I'm going to drop this patch.

I can see it causing more issues than it'll solve.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog