Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1256866 > unrolled thread
| Started by | Lee Jones <lee.jones@linaro.org> |
|---|---|
| First post | 2015-10-27 16:50 +0100 |
| Last post | 2015-10-28 10:00 +0100 |
| Articles | 9 on this page of 49 — 8 participants |
Back to article view | Back to linux.kernel
[PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag Lee Jones <lee.jones@linaro.org> - 2015-10-27 16:50 +0100
Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag Sebastian Reichel <sre@kernel.org> - 2015-10-27 18:30 +0100
Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag Lee Jones <lee.jones@linaro.org> - 2015-10-27 19:20 +0100
Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag Joe Perches <joe@perches.com> - 2015-10-27 19:50 +0100
Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2015-10-28 02:50 +0100
Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag Lee Jones <lee.jones@linaro.org> - 2015-10-28 09:40 +0100
Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag Javier Martinez Canillas <javier@dowhile0.org> - 2015-10-28 10:30 +0100
Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag Javier Martinez Canillas <javier@dowhile0.org> - 2015-10-28 10:30 +0100
Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2015-10-28 10:40 +0100
Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag Lee Jones <lee.jones@linaro.org> - 2015-10-28 11:30 +0100
Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag Javier Martinez Canillas <javier@dowhile0.org> - 2015-10-28 12:00 +0100
Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag Joe Perches <joe@perches.com> - 2015-10-28 12:10 +0100
Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag Javier Martinez Canillas <javier@dowhile0.org> - 2015-10-28 12:30 +0100
Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag Lee Jones <lee.jones@linaro.org> - 2015-10-28 12:40 +0100
Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag Lee Jones <lee.jones@linaro.org> - 2015-10-28 13:20 +0100
Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag Joe Perches <joe@perches.com> - 2015-10-28 13:30 +0100
Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag Lee Jones <lee.jones@linaro.org> - 2015-10-28 13:30 +0100
Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag Joe Perches <joe@perches.com> - 2015-10-28 13:50 +0100
Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag Javier Martinez Canillas <javier@dowhile0.org> - 2015-10-28 14:10 +0100
Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag Lee Jones <lee.jones@linaro.org> - 2015-10-28 14:40 +0100
Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag Lee Jones <lee.jones@linaro.org> - 2015-10-28 15:40 +0100
Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag Javier Martinez Canillas <javier@dowhile0.org> - 2015-10-28 16:00 +0100
Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2015-10-29 01:00 +0100
Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag Javier Martinez Canillas <javier@dowhile0.org> - 2015-10-29 01:20 +0100
Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag Lee Jones <lee.jones@linaro.org> - 2015-10-30 18:00 +0100
Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag Lee Jones <lee.jones@linaro.org> - 2015-10-30 18:00 +0100
Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag Javier Martinez Canillas <javier@dowhile0.org> - 2015-10-28 15:40 +0100
Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2015-10-28 10:30 +0100
Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag Uwe Kleine-König <u.kleine-koenig@pengutronix.de> - 2015-10-28 10:40 +0100
Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag Lee Jones <lee.jones@linaro.org> - 2015-10-28 11:00 +0100
Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2015-10-28 14:20 +0100
[PATCH] get_maintainer: Add subsystem to reviewer output Joe Perches <joe@perches.com> - 2015-10-28 17:50 +0100
Re: [PATCH] get_maintainer: Add subsystem to reviewer output Lee Jones <lee.jones@linaro.org> - 2015-10-28 18:10 +0100
Re: [PATCH] get_maintainer: Add subsystem to reviewer output Joe Perches <joe@perches.com> - 2015-10-28 18:10 +0100
Re: [PATCH] get_maintainer: Add subsystem to reviewer output Lee Jones <lee.jones@linaro.org> - 2015-10-28 18:30 +0100
Re: [PATCH] get_maintainer: Add subsystem to reviewer output Joe Perches <joe@perches.com> - 2015-10-28 18:40 +0100
Re: [PATCH] get_maintainer: Add subsystem to reviewer output Lee Jones <lee.jones@linaro.org> - 2015-10-28 18:50 +0100
Re: [PATCH] get_maintainer: Add subsystem to reviewer output Joe Perches <joe@perches.com> - 2015-10-28 19:00 +0100
Re: [PATCH] get_maintainer: Add subsystem to reviewer output Lee Jones <lee.jones@linaro.org> - 2015-10-29 10:30 +0100
Re: [PATCH] get_maintainer: Add subsystem to reviewer output Joe Perches <joe@perches.com> - 2015-10-29 15:20 +0100
Re: [PATCH] get_maintainer: Add subsystem to reviewer output Lee Jones <lee.jones@linaro.org> - 2015-10-29 17:20 +0100
Re: [PATCH] get_maintainer: Add subsystem to reviewer output Joe Perches <joe@perches.com> - 2015-10-28 18:20 +0100
Re: [PATCH] get_maintainer: Add subsystem to reviewer output Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2015-10-29 00:50 +0100
Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag Lee Jones <lee.jones@linaro.org> - 2015-10-28 11:20 +0100
Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2015-10-28 14:30 +0100
Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag Lee Jones <lee.jones@linaro.org> - 2015-10-28 14:50 +0100
Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> - 2015-10-28 17:30 +0100
Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag Lee Jones <lee.jones@linaro.org> - 2015-10-28 17:40 +0100
Re: [PATCH] MAINTAINERS: Start using the 'reviewer' (R) tag Chanwoo Choi <cw00.choi@samsung.com> - 2015-10-28 10:00 +0100
Page 3 of 3 — ← Prev page 1 2 [3]
| From | Lee Jones <lee.jones@linaro.org> |
|---|---|
| Date | 2015-10-29 17:20 +0100 |
| Subject | Re: [PATCH] get_maintainer: Add subsystem to reviewer output |
| Message-ID | <qoY5J-2tW-21@gated-at.bofh.it> |
| In reply to | #1258814 |
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Joe Perches <joe@perches.com> |
|---|---|
| Date | 2015-10-28 18:20 +0100 |
| Subject | Re: [PATCH] get_maintainer: Add subsystem to reviewer output |
| Message-ID | <qoCye-5zy-23@gated-at.bofh.it> |
| In reply to | #1258315 |
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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Krzysztof Kozlowski <k.kozlowski@samsung.com> |
|---|---|
| Date | 2015-10-29 00:50 +0100 |
| Subject | Re: [PATCH] get_maintainer: Add subsystem to reviewer output |
| Message-ID | <qoIDD-Si-3@gated-at.bofh.it> |
| In reply to | #1258298 |
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Lee Jones <lee.jones@linaro.org> |
|---|---|
| Date | 2015-10-28 11:20 +0100 |
| Message-ID | <qovZL-1i2-3@gated-at.bofh.it> |
| In reply to | #1257845 |
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Krzysztof Kozlowski <k.kozlowski@samsung.com> |
|---|---|
| Date | 2015-10-28 14:30 +0100 |
| Message-ID | <qoyXE-38X-31@gated-at.bofh.it> |
| In reply to | #1257880 |
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Lee Jones <lee.jones@linaro.org> |
|---|---|
| Date | 2015-10-28 14:50 +0100 |
| Message-ID | <qozgZ-3gf-15@gated-at.bofh.it> |
| In reply to | #1258023 |
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> |
|---|---|
| Date | 2015-10-28 17:30 +0100 |
| Message-ID | <qoBLQ-4Ye-19@gated-at.bofh.it> |
| In reply to | #1257797 |
[ 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Lee Jones <lee.jones@linaro.org> |
|---|---|
| Date | 2015-10-28 17:40 +0100 |
| Message-ID | <qoBVw-51M-35@gated-at.bofh.it> |
| In reply to | #1258287 |
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Chanwoo Choi <cw00.choi@samsung.com> |
|---|---|
| Date | 2015-10-28 10:00 +0100 |
| Message-ID | <qouKm-lA-25@gated-at.bofh.it> |
| In reply to | #1256866 |
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [standalone]
Page 3 of 3 — ← Prev page 1 2 [3]
Back to top | Article view | linux.kernel
csiph-web