MAINTAINERS: Start using the 'reviewer' (R) tag

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).

Quoted 8 lines · 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.

Quoted 3 lines

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 <[email protected]>

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

Quoted 8 lines · Sebastian Reichel

Quoted 7 lines · 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.

Quoted 6 lines

Quoted 3 lines

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 <[email protected]>

Thanks.

Quoted 2 lines

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.

Quoted 4 lines

On Tue, 27 Oct 2015, Sebastian Reichel wrote:> >

Quoted 2 lines

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

Quoted 3 lines

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.

Quoted 8 lines · Sebastian Reichel

Quoted 7 lines · 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.

2015-10-28 3:44 GMT+09:00 Joe Perches <[email protected]>:

Quoted 11 lines · Lee Jones

Quoted 7 lines

On Tue, 27 Oct 2015, Sebastian Reichel wrote:> >

Quoted 2 lines

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.

Quoted 1 line

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.

Quoted 9 lines · Lee Jones

Quoted 8 lines · Sebastian Reichel

Quoted 7 lines · 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.

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.

Quoted 12 lines · Krzysztof Kozlowski

2015-10-28 3:44 GMT+09:00 Joe Perches <[email protected]>:

Quoted 11 lines · Lee Jones

Quoted 7 lines

On Tue, 27 Oct 2015, Sebastian Reichel wrote:> >

Quoted 2 lines

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.

Quoted 2 lines

Quoted 1 line

Their drivers are not thrown over a wall and forgotten.

Quoted 5 lines

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.

Quoted 2 lines

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

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

Hello Lee,

Quoted 9 lines

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 <[email protected]> (supporter:MAXIM PMIC AND MUIC DRIVERS FOR EXYNOS BASED BO...)
Lee Jones <[email protected]> (supporter:MULTIFUNCTION DEVICES (MFD))

and after the change:

Krzysztof Kozlowski <[email protected]> (reviewer)
Lee Jones <[email protected]> (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).

Quoted 34 lines · Lee Jones

Quoted 9 lines · Lee Jones

Quoted 8 lines · Sebastian Reichel

Quoted 7 lines · 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.

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.

Quoted 12 lines · Krzysztof Kozlowski

2015-10-28 3:44 GMT+09:00 Joe Perches <[email protected]>:

Quoted 11 lines · Lee Jones

Quoted 7 lines

On Tue, 27 Oct 2015, Sebastian Reichel wrote:> >

Quoted 2 lines

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.

Quoted 12 lines

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.

Quoted 2 lines

Quoted 1 line

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.

Quoted 6 lines

Quoted 5 lines

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.

Quoted 4 lines

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

Quoted 2 lines

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.

Quoted 9 lines

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 <[email protected]> (supporter:MAXIM PMIC AND MUIC DRIVERS FOR EXYNOS BASED BO...)
Lee Jones <[email protected]> (supporter:MULTIFUNCTION DEVICES (MFD))

and after the change:

Krzysztof Kozlowski <[email protected]> (reviewer)
Lee Jones <[email protected]> (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).

Quoted 48 lines · Javier Martinez Canillas

Quoted 34 lines · Lee Jones

Quoted 9 lines · Lee Jones

Quoted 8 lines · Sebastian Reichel

Quoted 7 lines · 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.

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.

Quoted 12 lines · Krzysztof Kozlowski

2015-10-28 3:44 GMT+09:00 Joe Perches <[email protected]>:

Quoted 11 lines · Lee Jones

Quoted 7 lines

On Tue, 27 Oct 2015, Sebastian Reichel wrote:> >

Quoted 2 lines

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.

Quoted 12 lines

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.

Quoted 2 lines

Quoted 1 line

Their drivers are not thrown over a wall and forgotten.

Quoted 6 lines

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.

Quoted 6 lines

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

Quoted 9 lines

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.

Quoted 4 lines

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.

Quoted 8 lines

Quoted 8 lines

Quoted 5 lines

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.

Quoted 5 lines

Quoted 2 lines

Quoted 2 lines

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.

Quoted 22 lines

Quoted 9 lines

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 <[email protected]> (supporter:MAXIM PMIC AND MUIC DRIVERS FOR EXYNOS BASED BO...)
Lee Jones <[email protected]> (supporter:MULTIFUNCTION DEVICES (MFD))

and after the change:

Krzysztof Kozlowski <[email protected]> (reviewer)
Lee Jones <[email protected]> (supporter:MULTIFUNCTION DEVICES (MFD))

Quoted 3 lines

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

(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.

From MAINTAINERS:

Quoted 10 lines · Javier Martinez Canillas

(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

Hello Joe,

Quoted 22 lines · Joe Perches

Quoted 10 lines · Javier Martinez Canillas

(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.

Quoted 2 lines

"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.

Quoted 4 lines

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

Quoted 34 lines · Javier Martinez Canillas

Hello Joe,

Quoted 22 lines · Joe Perches

Quoted 10 lines · Javier Martinez Canillas

(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.

Quoted 2 lines

"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).

Quoted 6 lines

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.

Quoted 4 lines

Quoted 4 lines

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

Ah, but wait. get_maintainer.pl *does* assume M means Maintainer doesn't it?

Quoted 2 lines · Lee Jones

Ah, but wait. get_maintainer.pl *does* assume M means Maintainer doesn't it?

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

Quoted 5 lines · Joe Perches

Quoted 2 lines · Lee Jones

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).

Quoted 45 lines · Lee Jones

Quoted 34 lines · Javier Martinez Canillas

Hello Joe,

Quoted 22 lines · Joe Perches

Quoted 10 lines · Javier Martinez Canillas

(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.

Quoted 2 lines

"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).

Quoted 6 lines

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.

Quoted 8 lines

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?

Quoted 56 lines · Lee Jones

Quoted 45 lines · Lee Jones

Quoted 34 lines · Javier Martinez Canillas

Hello Joe,

Quoted 22 lines · Joe Perches

Quoted 10 lines · Javier Martinez Canillas

(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.

Quoted 2 lines

"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).

Quoted 6 lines

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.

Quoted 8 lines

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.

Quoted 3 lines

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.

Quoted 3 lines

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.

Quoted 7 lines

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.

Quoted 71 lines · Javier Martinez Canillas

Quoted 56 lines · Lee Jones

Quoted 45 lines · Lee Jones

Quoted 34 lines · Javier Martinez Canillas

Hello Joe,

Quoted 22 lines · Joe Perches

Quoted 10 lines · Javier Martinez Canillas

(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.

Quoted 2 lines

"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).

Quoted 6 lines

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.

Quoted 8 lines

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.

Quoted 3 lines

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".

Quoted 3 lines

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.

Quoted 11 lines

Quoted 3 lines

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. ;)

Quoted 7 lines

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?

Quoted 8 lines

Quoted 7 lines

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?

Quoted 2 lines

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.

Quoted 3 lines

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.

Quoted 4 lines

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; ;)

Quoted 76 lines · Lee Jones

Quoted 74 lines · Javier Martinez Canillas

Quoted 56 lines · Lee Jones

Quoted 45 lines · Lee Jones

Quoted 34 lines · Javier Martinez Canillas

Hello Joe,

Quoted 22 lines · Joe Perches

Quoted 10 lines · Javier Martinez Canillas

(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.

Quoted 2 lines

"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).

Quoted 6 lines

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.

Quoted 8 lines

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.

Quoted 3 lines

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.

Quoted 4 lines

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.

Quoted 12 lines

Quoted 11 lines

Quoted 3 lines

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.

Quoted 4 lines

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.

Quoted 9 lines

Quoted 7 lines

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.

Quoted 8 lines

Quoted 8 lines

Quoted 7 lines

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?

Quoted 8 lines

Quoted 2 lines

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.

Quoted 9 lines

Quoted 3 lines

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.

Quoted 12 lines

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.

Quoted 4 lines

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.

Quoted 2 lines

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 ;-)

Quoted 4 lines

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.

Quoted 2 lines · Javier Martinez Canillas

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.

Quoted 11 lines · Lee Jones

Quoted 2 lines · Javier Martinez Canillas

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 <[email protected]>

Best regards,
Krzysztof

Hello Lee,

Quoted 13 lines · Krzysztof Kozlowski

Quoted 11 lines · Lee Jones

Quoted 2 lines · Javier Martinez Canillas

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.

Quoted 8 lines

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 <[email protected]>

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 <[email protected]>

Quoted 11 lines · Lee Jones

Quoted 2 lines · Javier Martinez Canillas

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 <[email protected]>

Quoted 12 lines · Krzysztof Kozlowski

2015-10-28 3:44 GMT+09:00 Joe Perches <[email protected]>:

Quoted 11 lines · Lee Jones

Quoted 7 lines

On Tue, 27 Oct 2015, Sebastian Reichel wrote:> >

Quoted 2 lines

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.

Quoted 7 lines

Quoted 1 line

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.

Quoted 2 lines

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.

Quoted 3 lines · Lee Jones

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).

Hello,

Quoted 5 lines · Krzysztof Kozlowski

Quoted 3 lines · Lee Jones

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.

Quoted 4 lines

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?

Quoted 9 lines

Hello,

Quoted 5 lines · Krzysztof Kozlowski

Quoted 3 lines · Lee Jones

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.

Quoted 8 lines

Quoted 4 lines

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 <[email protected]> (maintainer:ARM/FREESCALE VYBRID ARM ARCHITECTURE)
 Sascha Hauer <[email protected]> (maintainer:ARM/FREESCALE VYBRID ARM ARCHITECTURE)
 Stefan Agner <[email protected]> (reviewer)
 Russell King <[email protected]> (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.

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 <[email protected]>

---
 scripts/get_maintainer.pl | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

scripts/get_maintainer.pl +16 -15

index 98bae86..d641541 100755
@@ -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);

Quoted 7 lines · 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

Quoted 4 lines

Signed-off-by Joe Perches <[email protected]>

---
 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 <[email protected]>

Quoted 14 lines · Lee Jones

Quoted 11 lines · 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 <[email protected]>

---
 scripts/get_maintainer.pl | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

Quoted 2 lines

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.

Quoted 6 lines

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 <[email protected]>

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

Quoted 2 lines · Joe Perches

Quoted 1 line

Acked-by Lee Jones <[email protected]>

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

Patch works as expected:

Tested-by Lee Jones <[email protected]>

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

Quoted 4 lines · Lee Jones

Quoted 2 lines · Joe Perches

Quoted 1 line

Acked-by Lee Jones <[email protected]>

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

Quoted 2 lines

Patch works as expected:

Tested-by Lee Jones <[email protected]>

[]

Quoted 2 lines

"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.

Quoted 7 lines · Joe Perches

Quoted 6 lines · Lee Jones

Quoted 2 lines · Joe Perches

Quoted 1 line

Acked-by Lee Jones <[email protected]>

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

Patch works as expected:

Tested-by Lee Jones <[email protected]>

Quoted 5 lines

[]

Quoted 2 lines

"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.

Quoted 12 lines · Lee Jones

Quoted 11 lines · Joe Perches

Quoted 6 lines · Lee Jones

Quoted 2 lines · Joe Perches

Quoted 1 line

Acked-by Lee Jones <[email protected]>

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

Patch works as expected:

Tested-by Lee Jones <[email protected]>

[]

Quoted 2 lines

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

Quoted 3 lines

Quoted 1 line

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.

Quoted 2 lines · Joe Perches

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 <[email protected]> (supporter:DRIVER SUBSYSTEM) Lee Jones <[email protected]> (reviewer:VENDOR DRIVER NAME)

Quoted 7 lines · Lee Jones

Quoted 2 lines · Joe Perches

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.

Quoted 4 lines

As an example:

Lee Jones <[email protected]> (supporter:DRIVER SUBSYSTEM) Lee Jones <[email protected]> (reviewer:VENDOR DRIVER NAME)

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

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.

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 <[email protected]>

---
 scripts/get_maintainer.pl | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

Quoted 17 lines · Lee Jones

Quoted 12 lines · Krzysztof Kozlowski

2015-10-28 3:44 GMT+09:00 Joe Perches <[email protected]>:

Quoted 11 lines · Lee Jones

Quoted 7 lines

On Tue, 27 Oct 2015, Sebastian Reichel wrote:> >

Quoted 2 lines

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.

Quoted 10 lines

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)?

Quoted 10 lines

Quoted 7 lines

Quoted 1 line

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.

Quoted 5 lines

Quoted 2 lines

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?

Quoted 3 lines

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.

Quoted 3 lines

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).

Quoted 29 lines · Lee Jones

Quoted 27 lines · Krzysztof Kozlowski

Quoted 12 lines

2015-10-28 3:44 GMT+09:00 Joe Perches <[email protected]>:

Quoted 11 lines · Lee Jones

Quoted 7 lines

On Tue, 27 Oct 2015, Sebastian Reichel wrote:> >

Quoted 2 lines

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.

Quoted 2 lines

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.

Quoted 2 lines

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*.

Quoted 2 lines

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).

Quoted 3 lines

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.

Quoted 13 lines

Quoted 13 lines

Quoted 7 lines

Quoted 1 line

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.

Quoted 2 lines

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

Quoted 4 lines

Quoted 2 lines

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.

Quoted 9 lines

Quoted 4 lines

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.

Quoted 5 lines

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.

Quoted 5 lines

Quoted 4 lines

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.

Quoted 4 lines

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.

W dniu 28.10.2015 o 19:14, Lee Jones pisze:

Quoted 30 lines · Krzysztof Kozlowski

Quoted 29 lines · Lee Jones

Quoted 27 lines · Krzysztof Kozlowski

Quoted 12 lines

2015-10-28 3:44 GMT+09:00 Joe Perches <[email protected]>:

Quoted 11 lines · Lee Jones

Quoted 7 lines

On Tue, 27 Oct 2015, Sebastian Reichel wrote:> >

Quoted 2 lines

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.

Quoted 5 lines

Quoted 2 lines

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?

Quoted 48 lines

Quoted 1 line

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*.

Quoted 2 lines

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).

Quoted 3 lines

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.

Quoted 17 lines

Quoted 8 lines

Quoted 7 lines

Quoted 1 line

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.

Quoted 7 lines

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

Quoted 2 lines

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?

Quoted 3 lines

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.

Quoted 20 lines

Quoted 8 lines

Quoted 3 lines

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.

Quoted 5 lines

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.

Quoted 3 lines

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.

Quoted 11 lines

Quoted 8 lines

Quoted 3 lines

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.

Quoted 9 lines · Lee Jones

Quoted 8 lines · Sebastian Reichel

Quoted 7 lines · 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.

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.

Quoted 12 lines · Krzysztof Kozlowski

2015-10-28 3:44 GMT+09:00 Joe Perches <[email protected]>:

Quoted 11 lines · Lee Jones

Quoted 7 lines

On Tue, 27 Oct 2015, Sebastian Reichel wrote:> >

Quoted 2 lines

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.

Quoted 7 lines

Quoted 1 line

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.

Quoted 2 lines

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.

Quoted 57 lines · Lee Jones

Quoted 9 lines · Lee Jones

Quoted 8 lines · Sebastian Reichel

Quoted 7 lines · 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.

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.

Quoted 12 lines · Krzysztof Kozlowski

2015-10-28 3:44 GMT+09:00 Joe Perches <[email protected]>:

Quoted 11 lines · Lee Jones

Quoted 7 lines

On Tue, 27 Oct 2015, Sebastian Reichel wrote:> >

Quoted 2 lines

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.

Quoted 7 lines

Quoted 1 line

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.

Quoted 2 lines

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

Quoted 8 lines

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.

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 <[email protected]>

---

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