* [Ksummit-discuss] crediting bug reports and fixes folded into original patch @ 2020-12-02 23:43 Vlastimil Babka 2020-12-03 4:02 ` Dan Williams ` (3 more replies) 0 siblings, 4 replies; 25+ messages in thread From: Vlastimil Babka @ 2020-12-02 23:43 UTC (permalink / raw) To: ksummit-discuss; +Cc: LKML Hi, there was a bit of debate on Twitter about this, so I thought I would bring it here. Imagine a scenario where patch sits as a commit in -next and there's a bug report or fix, possibly by a bot or with some static analysis. The maintainer decides to fold it into the original patch, which makes sense for e.g. bisectability. But there seem to be no clear rules about attribution in this case, which looks like there should be, probably in Documentation/maintainer/modifying-patches.rst The original bug fix might include a From: $author, a Reported-by: (e.g. syzbot), Fixes: $next-commit, some tag such as Addresses-Coverity: to credit the static analysis tool, and an SoB. After folding, all that's left might be a line as "include fix from $author" in the SoB area. This is a loss of metadata/attribution just due to folding, and might make contributors unhappy. Had they sent the fix after the original commit was mainline and immutable, all the info above would "survive" in the form of new commit. So I think we could decide what the proper format would be, and document it properly. I personally wouldn't mind just copy/pasting the whole commit message of the fix (with just a short issue description, no need to include stacktraces etc if the fix is folded), we could just standardize where, and how to delimit it from the main commit message. If it's a report (person or bot) of a bug that the main author then fixed, preserve the Reported-by in the same way (making clear it's not a Reported-By for the "main thing" addressed by the commit). In the debate one less verbose alternatve proposed was a SoB with comment describing it's for a fix and not whole patch, as some see SoB as the main mark of contribution, that can be easily found and counted etc. I'm not so sure about it myself, as AFAIK SoB is mainly a DCO thing, and for a maintainer it means something else ("passed through my tree") than for a patch author. And this approach would still lose the other tags. Thoughts? Vlastimil _______________________________________________ Ksummit-discuss mailing list Ksummit-discuss@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/ksummit-discuss ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Ksummit-discuss] crediting bug reports and fixes folded into original patch 2020-12-02 23:43 [Ksummit-discuss] crediting bug reports and fixes folded into original patch Vlastimil Babka @ 2020-12-03 4:02 ` Dan Williams 2020-12-03 9:34 ` Leon Romanovsky 2020-12-03 10:33 ` Dan Carpenter ` (2 subsequent siblings) 3 siblings, 1 reply; 25+ messages in thread From: Dan Williams @ 2020-12-03 4:02 UTC (permalink / raw) To: Vlastimil Babka; +Cc: LKML, ksummit-discuss On Wed, Dec 2, 2020 at 3:44 PM Vlastimil Babka <vbabka@suse.cz> wrote: > > Hi, > > there was a bit of debate on Twitter about this, so I thought I would bring it > here. Imagine a scenario where patch sits as a commit in -next and there's a bug > report or fix, possibly by a bot or with some static analysis. The maintainer > decides to fold it into the original patch, which makes sense for e.g. > bisectability. But there seem to be no clear rules about attribution in this > case, which looks like there should be, probably in > Documentation/maintainer/modifying-patches.rst > > The original bug fix might include a From: $author, a Reported-by: (e.g. > syzbot), Fixes: $next-commit, some tag such as Addresses-Coverity: to credit the > static analysis tool, and an SoB. After folding, all that's left might be a line > as "include fix from $author" in the SoB area. This is a loss of > metadata/attribution just due to folding, and might make contributors unhappy. > Had they sent the fix after the original commit was mainline and immutable, all > the info above would "survive" in the form of new commit. > > So I think we could decide what the proper format would be, and document it > properly. I personally wouldn't mind just copy/pasting the whole commit message > of the fix (with just a short issue description, no need to include stacktraces > etc if the fix is folded), we could just standardize where, and how to delimit > it from the main commit message. If it's a report (person or bot) of a bug that > the main author then fixed, preserve the Reported-by in the same way (making > clear it's not a Reported-By for the "main thing" addressed by the commit). > > In the debate one less verbose alternatve proposed was a SoB with comment > describing it's for a fix and not whole patch, as some see SoB as the main mark > of contribution, that can be easily found and counted etc. I'm not so sure about > it myself, as AFAIK SoB is mainly a DCO thing, and for a maintainer it means > something else ("passed through my tree") than for a patch author. And this > approach would still lose the other tags. > > Thoughts? How about a convention to add a Reported-by: and a Link: to the incremental fixup discussion? It's just polite to credit helpful feedback, not sure it needs a more formal process. Along those lines, how is this situation different than the feedback that helps improve a patch that does not necessarily get credited by Reviewed-by:? Links to thank you notes in cover letters seems more appealing than moving more review / fix logs into the main history. _______________________________________________ Ksummit-discuss mailing list Ksummit-discuss@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/ksummit-discuss ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Ksummit-discuss] crediting bug reports and fixes folded into original patch 2020-12-03 4:02 ` Dan Williams @ 2020-12-03 9:34 ` Leon Romanovsky 2020-12-03 9:36 ` Geert Uytterhoeven 0 siblings, 1 reply; 25+ messages in thread From: Leon Romanovsky @ 2020-12-03 9:34 UTC (permalink / raw) To: Dan Williams; +Cc: ksummit-discuss, LKML, Vlastimil Babka On Wed, Dec 02, 2020 at 08:02:27PM -0800, Dan Williams wrote: > On Wed, Dec 2, 2020 at 3:44 PM Vlastimil Babka <vbabka@suse.cz> wrote: > > > > Hi, > > > > there was a bit of debate on Twitter about this, so I thought I would bring it > > here. Imagine a scenario where patch sits as a commit in -next and there's a bug > > report or fix, possibly by a bot or with some static analysis. The maintainer > > decides to fold it into the original patch, which makes sense for e.g. > > bisectability. But there seem to be no clear rules about attribution in this > > case, which looks like there should be, probably in > > Documentation/maintainer/modifying-patches.rst > > > > The original bug fix might include a From: $author, a Reported-by: (e.g. > > syzbot), Fixes: $next-commit, some tag such as Addresses-Coverity: to credit the > > static analysis tool, and an SoB. After folding, all that's left might be a line > > as "include fix from $author" in the SoB area. This is a loss of > > metadata/attribution just due to folding, and might make contributors unhappy. > > Had they sent the fix after the original commit was mainline and immutable, all > > the info above would "survive" in the form of new commit. > > > > So I think we could decide what the proper format would be, and document it > > properly. I personally wouldn't mind just copy/pasting the whole commit message > > of the fix (with just a short issue description, no need to include stacktraces > > etc if the fix is folded), we could just standardize where, and how to delimit > > it from the main commit message. If it's a report (person or bot) of a bug that > > the main author then fixed, preserve the Reported-by in the same way (making > > clear it's not a Reported-By for the "main thing" addressed by the commit). > > > > In the debate one less verbose alternatve proposed was a SoB with comment > > describing it's for a fix and not whole patch, as some see SoB as the main mark > > of contribution, that can be easily found and counted etc. I'm not so sure about > > it myself, as AFAIK SoB is mainly a DCO thing, and for a maintainer it means > > something else ("passed through my tree") than for a patch author. And this > > approach would still lose the other tags. > > > > Thoughts? > > How about a convention to add a Reported-by: and a Link: to the > incremental fixup discussion? It's just polite to credit helpful > feedback, not sure it needs a more formal process. Maybe "Fixup-Reported-by:" and "Fixup-Link:"? Thanks _______________________________________________ Ksummit-discuss mailing list Ksummit-discuss@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/ksummit-discuss ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Ksummit-discuss] crediting bug reports and fixes folded into original patch 2020-12-03 9:34 ` Leon Romanovsky @ 2020-12-03 9:36 ` Geert Uytterhoeven 2020-12-03 10:40 ` Leon Romanovsky 0 siblings, 1 reply; 25+ messages in thread From: Geert Uytterhoeven @ 2020-12-03 9:36 UTC (permalink / raw) To: Leon Romanovsky; +Cc: Vlastimil Babka, LKML, ksummit-discuss On Thu, Dec 3, 2020 at 10:35 AM Leon Romanovsky <leon@kernel.org> wrote: > On Wed, Dec 02, 2020 at 08:02:27PM -0800, Dan Williams wrote: > > On Wed, Dec 2, 2020 at 3:44 PM Vlastimil Babka <vbabka@suse.cz> wrote: > > > there was a bit of debate on Twitter about this, so I thought I would bring it > > > here. Imagine a scenario where patch sits as a commit in -next and there's a bug > > > report or fix, possibly by a bot or with some static analysis. The maintainer > > > decides to fold it into the original patch, which makes sense for e.g. > > > bisectability. But there seem to be no clear rules about attribution in this > > > case, which looks like there should be, probably in > > > Documentation/maintainer/modifying-patches.rst > > > > > > The original bug fix might include a From: $author, a Reported-by: (e.g. > > > syzbot), Fixes: $next-commit, some tag such as Addresses-Coverity: to credit the > > > static analysis tool, and an SoB. After folding, all that's left might be a line > > > as "include fix from $author" in the SoB area. This is a loss of > > > metadata/attribution just due to folding, and might make contributors unhappy. > > > Had they sent the fix after the original commit was mainline and immutable, all > > > the info above would "survive" in the form of new commit. > > > > > > So I think we could decide what the proper format would be, and document it > > > properly. I personally wouldn't mind just copy/pasting the whole commit message > > > of the fix (with just a short issue description, no need to include stacktraces > > > etc if the fix is folded), we could just standardize where, and how to delimit > > > it from the main commit message. If it's a report (person or bot) of a bug that > > > the main author then fixed, preserve the Reported-by in the same way (making > > > clear it's not a Reported-By for the "main thing" addressed by the commit). > > > > > > In the debate one less verbose alternatve proposed was a SoB with comment > > > describing it's for a fix and not whole patch, as some see SoB as the main mark > > > of contribution, that can be easily found and counted etc. I'm not so sure about > > > it myself, as AFAIK SoB is mainly a DCO thing, and for a maintainer it means > > > something else ("passed through my tree") than for a patch author. And this > > > approach would still lose the other tags. > > > > > > Thoughts? > > > > How about a convention to add a Reported-by: and a Link: to the > > incremental fixup discussion? It's just polite to credit helpful > > feedback, not sure it needs a more formal process. > > Maybe "Fixup-Reported-by:" and "Fixup-Link:"? And "Earlier-Review-Comments-Provided-by:"? How far do we want to go? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds _______________________________________________ Ksummit-discuss mailing list Ksummit-discuss@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/ksummit-discuss ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Ksummit-discuss] crediting bug reports and fixes folded into original patch 2020-12-03 9:36 ` Geert Uytterhoeven @ 2020-12-03 10:40 ` Leon Romanovsky 2020-12-03 18:30 ` Greg KH 0 siblings, 1 reply; 25+ messages in thread From: Leon Romanovsky @ 2020-12-03 10:40 UTC (permalink / raw) To: Geert Uytterhoeven; +Cc: Vlastimil Babka, LKML, ksummit-discuss On Thu, Dec 03, 2020 at 10:36:56AM +0100, Geert Uytterhoeven wrote: > On Thu, Dec 3, 2020 at 10:35 AM Leon Romanovsky <leon@kernel.org> wrote: > > On Wed, Dec 02, 2020 at 08:02:27PM -0800, Dan Williams wrote: > > > On Wed, Dec 2, 2020 at 3:44 PM Vlastimil Babka <vbabka@suse.cz> wrote: > > > > there was a bit of debate on Twitter about this, so I thought I would bring it > > > > here. Imagine a scenario where patch sits as a commit in -next and there's a bug > > > > report or fix, possibly by a bot or with some static analysis. The maintainer > > > > decides to fold it into the original patch, which makes sense for e.g. > > > > bisectability. But there seem to be no clear rules about attribution in this > > > > case, which looks like there should be, probably in > > > > Documentation/maintainer/modifying-patches.rst > > > > > > > > The original bug fix might include a From: $author, a Reported-by: (e.g. > > > > syzbot), Fixes: $next-commit, some tag such as Addresses-Coverity: to credit the > > > > static analysis tool, and an SoB. After folding, all that's left might be a line > > > > as "include fix from $author" in the SoB area. This is a loss of > > > > metadata/attribution just due to folding, and might make contributors unhappy. > > > > Had they sent the fix after the original commit was mainline and immutable, all > > > > the info above would "survive" in the form of new commit. > > > > > > > > So I think we could decide what the proper format would be, and document it > > > > properly. I personally wouldn't mind just copy/pasting the whole commit message > > > > of the fix (with just a short issue description, no need to include stacktraces > > > > etc if the fix is folded), we could just standardize where, and how to delimit > > > > it from the main commit message. If it's a report (person or bot) of a bug that > > > > the main author then fixed, preserve the Reported-by in the same way (making > > > > clear it's not a Reported-By for the "main thing" addressed by the commit). > > > > > > > > In the debate one less verbose alternatve proposed was a SoB with comment > > > > describing it's for a fix and not whole patch, as some see SoB as the main mark > > > > of contribution, that can be easily found and counted etc. I'm not so sure about > > > > it myself, as AFAIK SoB is mainly a DCO thing, and for a maintainer it means > > > > something else ("passed through my tree") than for a patch author. And this > > > > approach would still lose the other tags. > > > > > > > > Thoughts? > > > > > > How about a convention to add a Reported-by: and a Link: to the > > > incremental fixup discussion? It's just polite to credit helpful > > > feedback, not sure it needs a more formal process. > > > > Maybe "Fixup-Reported-by:" and "Fixup-Link:"? > > And "Earlier-Review-Comments-Provided-by:"? > > How far do we want to go? I don't want to overload existing meaning of "Reported-by:" and "Link:", so anything else is fine by me. I imagine that all those who puts their own Reviewed-by, Signed-off-by and Tested-by in the same patch will be happy to use something like you are proposing - "Co-developed-Signed-Reviewed-Tested-by:" tag. Thanks > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds _______________________________________________ Ksummit-discuss mailing list Ksummit-discuss@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/ksummit-discuss ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Ksummit-discuss] crediting bug reports and fixes folded into original patch 2020-12-03 10:40 ` Leon Romanovsky @ 2020-12-03 18:30 ` Greg KH 2020-12-03 19:04 ` Leon Romanovsky 2020-12-09 0:34 ` Kees Cook 0 siblings, 2 replies; 25+ messages in thread From: Greg KH @ 2020-12-03 18:30 UTC (permalink / raw) To: Leon Romanovsky; +Cc: ksummit-discuss, LKML, Vlastimil Babka On Thu, Dec 03, 2020 at 12:40:47PM +0200, Leon Romanovsky wrote: > On Thu, Dec 03, 2020 at 10:36:56AM +0100, Geert Uytterhoeven wrote: > > On Thu, Dec 3, 2020 at 10:35 AM Leon Romanovsky <leon@kernel.org> wrote: > > > On Wed, Dec 02, 2020 at 08:02:27PM -0800, Dan Williams wrote: > > > > On Wed, Dec 2, 2020 at 3:44 PM Vlastimil Babka <vbabka@suse.cz> wrote: > > > > > there was a bit of debate on Twitter about this, so I thought I would bring it > > > > > here. Imagine a scenario where patch sits as a commit in -next and there's a bug > > > > > report or fix, possibly by a bot or with some static analysis. The maintainer > > > > > decides to fold it into the original patch, which makes sense for e.g. > > > > > bisectability. But there seem to be no clear rules about attribution in this > > > > > case, which looks like there should be, probably in > > > > > Documentation/maintainer/modifying-patches.rst > > > > > > > > > > The original bug fix might include a From: $author, a Reported-by: (e.g. > > > > > syzbot), Fixes: $next-commit, some tag such as Addresses-Coverity: to credit the > > > > > static analysis tool, and an SoB. After folding, all that's left might be a line > > > > > as "include fix from $author" in the SoB area. This is a loss of > > > > > metadata/attribution just due to folding, and might make contributors unhappy. > > > > > Had they sent the fix after the original commit was mainline and immutable, all > > > > > the info above would "survive" in the form of new commit. > > > > > > > > > > So I think we could decide what the proper format would be, and document it > > > > > properly. I personally wouldn't mind just copy/pasting the whole commit message > > > > > of the fix (with just a short issue description, no need to include stacktraces > > > > > etc if the fix is folded), we could just standardize where, and how to delimit > > > > > it from the main commit message. If it's a report (person or bot) of a bug that > > > > > the main author then fixed, preserve the Reported-by in the same way (making > > > > > clear it's not a Reported-By for the "main thing" addressed by the commit). > > > > > > > > > > In the debate one less verbose alternatve proposed was a SoB with comment > > > > > describing it's for a fix and not whole patch, as some see SoB as the main mark > > > > > of contribution, that can be easily found and counted etc. I'm not so sure about > > > > > it myself, as AFAIK SoB is mainly a DCO thing, and for a maintainer it means > > > > > something else ("passed through my tree") than for a patch author. And this > > > > > approach would still lose the other tags. > > > > > > > > > > Thoughts? > > > > > > > > How about a convention to add a Reported-by: and a Link: to the > > > > incremental fixup discussion? It's just polite to credit helpful > > > > feedback, not sure it needs a more formal process. > > > > > > Maybe "Fixup-Reported-by:" and "Fixup-Link:"? > > > > And "Earlier-Review-Comments-Provided-by:"? > > > > How far do we want to go? > > I don't want to overload existing meaning of "Reported-by:" and "Link:", > so anything else is fine by me. > > I imagine that all those who puts their own Reviewed-by, Signed-off-by > and Tested-by in the same patch will be happy to use something like you > are proposing - "Co-developed-Signed-Reviewed-Tested-by:" tag. We already have "Co-developerd-by:" as a valid tag, no need to merge more into this :) _______________________________________________ Ksummit-discuss mailing list Ksummit-discuss@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/ksummit-discuss ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Ksummit-discuss] crediting bug reports and fixes folded into original patch 2020-12-03 18:30 ` Greg KH @ 2020-12-03 19:04 ` Leon Romanovsky 2020-12-09 0:34 ` Kees Cook 1 sibling, 0 replies; 25+ messages in thread From: Leon Romanovsky @ 2020-12-03 19:04 UTC (permalink / raw) To: Greg KH; +Cc: ksummit-discuss, LKML, Vlastimil Babka On Thu, Dec 03, 2020 at 07:30:44PM +0100, Greg KH wrote: > On Thu, Dec 03, 2020 at 12:40:47PM +0200, Leon Romanovsky wrote: > > On Thu, Dec 03, 2020 at 10:36:56AM +0100, Geert Uytterhoeven wrote: > > > On Thu, Dec 3, 2020 at 10:35 AM Leon Romanovsky <leon@kernel.org> wrote: > > > > On Wed, Dec 02, 2020 at 08:02:27PM -0800, Dan Williams wrote: > > > > > On Wed, Dec 2, 2020 at 3:44 PM Vlastimil Babka <vbabka@suse.cz> wrote: > > > > > > there was a bit of debate on Twitter about this, so I thought I would bring it > > > > > > here. Imagine a scenario where patch sits as a commit in -next and there's a bug > > > > > > report or fix, possibly by a bot or with some static analysis. The maintainer > > > > > > decides to fold it into the original patch, which makes sense for e.g. > > > > > > bisectability. But there seem to be no clear rules about attribution in this > > > > > > case, which looks like there should be, probably in > > > > > > Documentation/maintainer/modifying-patches.rst > > > > > > > > > > > > The original bug fix might include a From: $author, a Reported-by: (e.g. > > > > > > syzbot), Fixes: $next-commit, some tag such as Addresses-Coverity: to credit the > > > > > > static analysis tool, and an SoB. After folding, all that's left might be a line > > > > > > as "include fix from $author" in the SoB area. This is a loss of > > > > > > metadata/attribution just due to folding, and might make contributors unhappy. > > > > > > Had they sent the fix after the original commit was mainline and immutable, all > > > > > > the info above would "survive" in the form of new commit. > > > > > > > > > > > > So I think we could decide what the proper format would be, and document it > > > > > > properly. I personally wouldn't mind just copy/pasting the whole commit message > > > > > > of the fix (with just a short issue description, no need to include stacktraces > > > > > > etc if the fix is folded), we could just standardize where, and how to delimit > > > > > > it from the main commit message. If it's a report (person or bot) of a bug that > > > > > > the main author then fixed, preserve the Reported-by in the same way (making > > > > > > clear it's not a Reported-By for the "main thing" addressed by the commit). > > > > > > > > > > > > In the debate one less verbose alternatve proposed was a SoB with comment > > > > > > describing it's for a fix and not whole patch, as some see SoB as the main mark > > > > > > of contribution, that can be easily found and counted etc. I'm not so sure about > > > > > > it myself, as AFAIK SoB is mainly a DCO thing, and for a maintainer it means > > > > > > something else ("passed through my tree") than for a patch author. And this > > > > > > approach would still lose the other tags. > > > > > > > > > > > > Thoughts? > > > > > > > > > > How about a convention to add a Reported-by: and a Link: to the > > > > > incremental fixup discussion? It's just polite to credit helpful > > > > > feedback, not sure it needs a more formal process. > > > > > > > > Maybe "Fixup-Reported-by:" and "Fixup-Link:"? > > > > > > And "Earlier-Review-Comments-Provided-by:"? > > > > > > How far do we want to go? > > > > I don't want to overload existing meaning of "Reported-by:" and "Link:", > > so anything else is fine by me. > > > > I imagine that all those who puts their own Reviewed-by, Signed-off-by > > and Tested-by in the same patch will be happy to use something like you > > are proposing - "Co-developed-Signed-Reviewed-Tested-by:" tag. > > We already have "Co-developerd-by:" as a valid tag, no need to merge > more into this :) It was joke, but the reality is even more exciting. See commit 71cc849b7093 ("KVM: x86: Fix split-irqchip vs interrupt injection window request") for the need of "Reported-Analyzed-Reviewed-Tested-by:" tag. And endless amount of commits with "Reviewed-Signed-by:" from maintainers that gives wrong impression that other maintainers merge code without reviewing it. Thanks _______________________________________________ Ksummit-discuss mailing list Ksummit-discuss@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/ksummit-discuss ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Ksummit-discuss] crediting bug reports and fixes folded into original patch 2020-12-03 18:30 ` Greg KH 2020-12-03 19:04 ` Leon Romanovsky @ 2020-12-09 0:34 ` Kees Cook 2020-12-09 5:01 ` Joe Perches 1 sibling, 1 reply; 25+ messages in thread From: Kees Cook @ 2020-12-09 0:34 UTC (permalink / raw) To: Greg KH Cc: ksummit-discuss, LKML, Dan Carpenter, Colin Ian King, Vlastimil Babka On Thu, Dec 03, 2020 at 07:30:44PM +0100, Greg KH wrote: > On Thu, Dec 03, 2020 at 12:40:47PM +0200, Leon Romanovsky wrote: > > On Thu, Dec 03, 2020 at 10:36:56AM +0100, Geert Uytterhoeven wrote: > > > On Thu, Dec 3, 2020 at 10:35 AM Leon Romanovsky <leon@kernel.org> wrote: > > > > On Wed, Dec 02, 2020 at 08:02:27PM -0800, Dan Williams wrote: > > > > > On Wed, Dec 2, 2020 at 3:44 PM Vlastimil Babka <vbabka@suse.cz> wrote: > > > > > > there was a bit of debate on Twitter about this, so I thought I would bring it > > > > > > here. Imagine a scenario where patch sits as a commit in -next and there's a bug > > > > > > report or fix, possibly by a bot or with some static analysis. The maintainer > > > > > > decides to fold it into the original patch, which makes sense for e.g. > > > > > > bisectability. But there seem to be no clear rules about attribution in this > > > > > > case, which looks like there should be, probably in > > > > > > Documentation/maintainer/modifying-patches.rst > > > > > > > > > > > > The original bug fix might include a From: $author, a Reported-by: (e.g. > > > > > > syzbot), Fixes: $next-commit, some tag such as Addresses-Coverity: to credit the > > > > > > static analysis tool, and an SoB. After folding, all that's left might be a line > > > > > > as "include fix from $author" in the SoB area. This is a loss of > > > > > > metadata/attribution just due to folding, and might make contributors unhappy. > > > > > > Had they sent the fix after the original commit was mainline and immutable, all > > > > > > the info above would "survive" in the form of new commit. > > > > > > > > > > > > So I think we could decide what the proper format would be, and document it > > > > > > properly. I personally wouldn't mind just copy/pasting the whole commit message > > > > > > of the fix (with just a short issue description, no need to include stacktraces > > > > > > etc if the fix is folded), we could just standardize where, and how to delimit > > > > > > it from the main commit message. If it's a report (person or bot) of a bug that > > > > > > the main author then fixed, preserve the Reported-by in the same way (making > > > > > > clear it's not a Reported-By for the "main thing" addressed by the commit). > > > > > > > > > > > > In the debate one less verbose alternatve proposed was a SoB with comment > > > > > > describing it's for a fix and not whole patch, as some see SoB as the main mark > > > > > > of contribution, that can be easily found and counted etc. I'm not so sure about > > > > > > it myself, as AFAIK SoB is mainly a DCO thing, and for a maintainer it means > > > > > > something else ("passed through my tree") than for a patch author. And this > > > > > > approach would still lose the other tags. > > > > > > > > > > > > Thoughts? > > > > > > > > > > How about a convention to add a Reported-by: and a Link: to the > > > > > incremental fixup discussion? It's just polite to credit helpful > > > > > feedback, not sure it needs a more formal process. To me, "Reported-by:" is associated with "this person reported the problem being fixed by this commit". For these kinds of larger commits, that may not be sensible. I.e. it's some larger feature that the "I found a problem with this commit" author wasn't even involved in. I think it's important to capture those in some way, even if they're not considered "copyrightable" or whatever -- that's not the bar I'm interested in; I want to make sure people are acknowledged for the time and effort they spent. And whether we like it or not, these kinds of tags do that. Besides, such fix authors have expressly asked for this, which should be sufficient reason enough to find a solution. > > > > Maybe "Fixup-Reported-by:" and "Fixup-Link:"? > > > > > > And "Earlier-Review-Comments-Provided-by:"? > > > > > > How far do we want to go? > > > > I don't want to overload existing meaning of "Reported-by:" and "Link:", > > so anything else is fine by me. Agreed. > > I imagine that all those who puts their own Reviewed-by, Signed-off-by > > and Tested-by in the same patch will be happy to use something like you > > are proposing - "Co-developed-Signed-Reviewed-Tested-by:" tag. > > We already have "Co-developerd-by:" as a valid tag, no need to merge > more into this :) "Co-developed-by", to me, has a connotation of significant authorship. For the "weaker" cases, I tend to use "Suggested-by" or put something like "Based on a patch by $person[link]" in the body. For the kinds of fixes mentioned here, and more specifically for the kinds of fixes that I have received from both Colin Ian King and Dan Carpenter that fall into this "tiny fix"[1] category, I think something simply like "Adjusted-by" could be used. I've already tried to include "Link" tags to things that got folded in, but without the Adjusted-by tag, it lacks the right kind of searchability and recognition. "Fixes-by" is too close to "Fixes" (and implies more than one fix). "Fixup-by" implies singular. "Debugged-by" is like the other existing high-level tags, in that they speak to the ENTIRE patch. If not "Adjusted-by", what about "Tweaked-by", "Helped-by", "Corrected-by"? Colin, Dan, any thoughts on how you'd like to see stuff? -Kees [1] "tiny" in the sense of characters changed, usually. There was very much NOT a "tiny" amount of time spent on it, nor do they have "tiny" impact -- which is the whole point of calling this out in the commit. -- Kees Cook _______________________________________________ Ksummit-discuss mailing list Ksummit-discuss@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/ksummit-discuss ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Ksummit-discuss] crediting bug reports and fixes folded into original patch 2020-12-09 0:34 ` Kees Cook @ 2020-12-09 5:01 ` Joe Perches 2020-12-09 7:58 ` Dan Carpenter 0 siblings, 1 reply; 25+ messages in thread From: Joe Perches @ 2020-12-09 5:01 UTC (permalink / raw) To: Kees Cook, Greg KH Cc: ksummit-discuss, LKML, Dan Carpenter, Colin Ian King, Vlastimil Babka On Tue, 2020-12-08 at 16:34 -0800, Kees Cook wrote: > If not "Adjusted-by", what about "Tweaked-by", "Helped-by", > "Corrected-by"? Improved-by: / Enhanced-by: / Revisions-by: Or simply don't use anything but a link to the conversion thread like Konstantin suggested. I still want to know what actual value these things have and to whom. _______________________________________________ Ksummit-discuss mailing list Ksummit-discuss@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/ksummit-discuss ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Ksummit-discuss] crediting bug reports and fixes folded into original patch 2020-12-09 5:01 ` Joe Perches @ 2020-12-09 7:58 ` Dan Carpenter 2020-12-09 8:45 ` Vlastimil Babka 2020-12-09 8:54 ` Joe Perches 0 siblings, 2 replies; 25+ messages in thread From: Dan Carpenter @ 2020-12-09 7:58 UTC (permalink / raw) To: Joe Perches Cc: ksummit-discuss, Greg KH, LKML, Colin Ian King, Vlastimil Babka On Tue, Dec 08, 2020 at 09:01:49PM -0800, Joe Perches wrote: > On Tue, 2020-12-08 at 16:34 -0800, Kees Cook wrote: > > > If not "Adjusted-by", what about "Tweaked-by", "Helped-by", > > "Corrected-by"? > > Improved-by: / Enhanced-by: / Revisions-by: > I don't think we should give any credit for improvements or enhancements, only for fixes. Complaining about style is its own reward. Having to redo a patch is already a huge headache. Normally, I already considered the reviewer's prefered style and decided I didn't like it. Then to make me redo the patch in an ugly style and say thankyou on top of that??? Forget about it. Plus, as a reviewer I hate reviewing patches over and over. I've argued for years that we should have a Fixes-from: tag. The zero day bot is already encouraging people to add Reported-by tags for this and a lot of people do. regards, dan carpenter _______________________________________________ Ksummit-discuss mailing list Ksummit-discuss@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/ksummit-discuss ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Ksummit-discuss] crediting bug reports and fixes folded into original patch 2020-12-09 7:58 ` Dan Carpenter @ 2020-12-09 8:45 ` Vlastimil Babka 2020-12-09 9:18 ` Geert Uytterhoeven 2020-12-09 8:54 ` Joe Perches 1 sibling, 1 reply; 25+ messages in thread From: Vlastimil Babka @ 2020-12-09 8:45 UTC (permalink / raw) To: Dan Carpenter, Joe Perches; +Cc: Greg KH, Colin Ian King, ksummit-discuss, LKML On 12/9/20 8:58 AM, Dan Carpenter wrote: > On Tue, Dec 08, 2020 at 09:01:49PM -0800, Joe Perches wrote: >> On Tue, 2020-12-08 at 16:34 -0800, Kees Cook wrote: >> >> > If not "Adjusted-by", what about "Tweaked-by", "Helped-by", >> > "Corrected-by"? >> >> Improved-by: / Enhanced-by: / Revisions-by: >> > > I don't think we should give any credit for improvements or enhancements, Well, some are actually useful and not about reviewer's preferred style :) But if an author redoes the patch as a result, it's their choice to mention useful improvements in the next version's change log. > only for fixes. Complaining about style is its own reward. Right, let's focus on fixes and reports of bugs, that would have resulted in a standalone commit, but don't. > Having to redo a patch is already a huge headache. Normally, I already > considered the reviewer's prefered style and decided I didn't like it. > Then to make me redo the patch in an ugly style and say thankyou on > top of that??? Forget about it. Plus, as a reviewer I hate reviewing > patches over and over. > > I've argued for years that we should have a Fixes-from: tag. The zero Standardizing the Fixes-from: tag (or any better name) would be a forward progress, yes. > day bot is already encouraging people to add Reported-by tags for this > and a lot of people do. "Reported-by:" becomes ambiguous once the bugfix for the reported issue in the patch is folded, as it's no longer clear whether the bot reported the original issue the patch is fixing, or a bug in the fix. So we should have a different variant. "Fixes-reported-by:" so it has the same prefix? > regards, > dan carpenter > _______________________________________________ Ksummit-discuss mailing list Ksummit-discuss@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/ksummit-discuss ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Ksummit-discuss] crediting bug reports and fixes folded into original patch 2020-12-09 8:45 ` Vlastimil Babka @ 2020-12-09 9:18 ` Geert Uytterhoeven 0 siblings, 0 replies; 25+ messages in thread From: Geert Uytterhoeven @ 2020-12-09 9:18 UTC (permalink / raw) To: Vlastimil Babka Cc: ksummit-discuss, Greg KH, LKML, Colin Ian King, Dan Carpenter On Wed, Dec 9, 2020 at 9:45 AM Vlastimil Babka <vbabka@suse.cz> wrote: > On 12/9/20 8:58 AM, Dan Carpenter wrote: > > On Tue, Dec 08, 2020 at 09:01:49PM -0800, Joe Perches wrote: > >> On Tue, 2020-12-08 at 16:34 -0800, Kees Cook wrote: > >> > >> > If not "Adjusted-by", what about "Tweaked-by", "Helped-by", > >> > "Corrected-by"? > >> > >> Improved-by: / Enhanced-by: / Revisions-by: > >> > > > > I don't think we should give any credit for improvements or enhancements, > > Well, some are actually useful and not about reviewer's preferred style :) But > if an author redoes the patch as a result, it's their choice to mention useful > improvements in the next version's change log. > > > only for fixes. Complaining about style is its own reward. > > Right, let's focus on fixes and reports of bugs, that would have resulted in a > standalone commit, but don't. > > > Having to redo a patch is already a huge headache. Normally, I already > > considered the reviewer's prefered style and decided I didn't like it. > > Then to make me redo the patch in an ugly style and say thankyou on > > top of that??? Forget about it. Plus, as a reviewer I hate reviewing > > patches over and over. > > > > I've argued for years that we should have a Fixes-from: tag. The zero > > Standardizing the Fixes-from: tag (or any better name) would be a forward > progress, yes. > > > day bot is already encouraging people to add Reported-by tags for this > > and a lot of people do. > > "Reported-by:" becomes ambiguous once the bugfix for the reported issue in the > patch is folded, as it's no longer clear whether the bot reported the original > issue the patch is fixing, or a bug in the fix. So we should have a different > variant. "Fixes-reported-by:" so it has the same prefix? Taken-into-account-comments-from: Any terse English word for that? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds _______________________________________________ Ksummit-discuss mailing list Ksummit-discuss@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/ksummit-discuss ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Ksummit-discuss] crediting bug reports and fixes folded into original patch 2020-12-09 7:58 ` Dan Carpenter 2020-12-09 8:45 ` Vlastimil Babka @ 2020-12-09 8:54 ` Joe Perches 2020-12-09 10:30 ` Dan Carpenter 1 sibling, 1 reply; 25+ messages in thread From: Joe Perches @ 2020-12-09 8:54 UTC (permalink / raw) To: Dan Carpenter Cc: ksummit-discuss, Greg KH, LKML, Colin Ian King, Vlastimil Babka On Wed, 2020-12-09 at 10:58 +0300, Dan Carpenter wrote: > On Tue, Dec 08, 2020 at 09:01:49PM -0800, Joe Perches wrote: > > On Tue, 2020-12-08 at 16:34 -0800, Kees Cook wrote: > > > > > If not "Adjusted-by", what about "Tweaked-by", "Helped-by", > > > "Corrected-by"? > > > > Improved-by: / Enhanced-by: / Revisions-by: > > > > I don't think we should give any credit for improvements or enhancements, > only for fixes. Hey Dan. I do. If a patch isn't comprehensive and a reviewer notices some missing coverage or algorithmic performance enhancement, I think that should be noted. > Complaining about style is its own reward. <chuckle, maybe so. I view it more like coaching...> I believe I've said multiple times that style changes shouldn't require additional commentary added to a patch. I'm not making any suggestion to comment for style, only logic or defect reduction/improvements as described above. > Having to redo a patch is already a huge headache. Normally, I already > considered the reviewer's prefered style and decided I didn't like it. Example please. We both seem to prefer consistent style. > Then to make me redo the patch in an ugly style and say thank you on > top of that??? Forget about it. Not a thing I've asked for. > Plus, as a reviewer I hate reviewing patches over and over. interdiff could be improved. > I've argued for years that we should have a Fixes-from: tag. The zero > day bot is already encouraging people to add Reported-by tags for this > and a lot of people do. It's still a question of what fixes means in any context. https://www.google.com/search?q=%27fixes-from%3A%27%20carpenter%20site%3Alore.kernel.org gives: It looks like there aren't many great matches for your search And I'm rather in favor of letting people make up their own <whatever>-by: uses and not being too concerned about the specific whatever word or phrase used. Postel's law and such. _______________________________________________ Ksummit-discuss mailing list Ksummit-discuss@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/ksummit-discuss ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Ksummit-discuss] crediting bug reports and fixes folded into original patch 2020-12-09 8:54 ` Joe Perches @ 2020-12-09 10:30 ` Dan Carpenter 2020-12-09 17:45 ` Dan Williams 0 siblings, 1 reply; 25+ messages in thread From: Dan Carpenter @ 2020-12-09 10:30 UTC (permalink / raw) To: Joe Perches Cc: Vlastimil Babka, Greg KH, LKML, ksummit-discuss, Colin Ian King On Wed, Dec 09, 2020 at 12:54:30AM -0800, Joe Perches wrote: > On Wed, 2020-12-09 at 10:58 +0300, Dan Carpenter wrote: > > On Tue, Dec 08, 2020 at 09:01:49PM -0800, Joe Perches wrote: > > > On Tue, 2020-12-08 at 16:34 -0800, Kees Cook wrote: > > > > > > > If not "Adjusted-by", what about "Tweaked-by", "Helped-by", > > > > "Corrected-by"? > > > > > > Improved-by: / Enhanced-by: / Revisions-by: > > > > > > > I don't think we should give any credit for improvements or enhancements, > > only for fixes. > > Hey Dan. > > I do. If a patch isn't comprehensive and a reviewer notices some > missing coverage or algorithmic performance enhancement, I think that > should be noted. > > > Complaining about style is its own reward. > > <chuckle, maybe so. I view it more like coaching...> > > I believe I've said multiple times that style changes shouldn't require > additional commentary added to a patch. > > I'm not making any suggestion to comment for style, only logic or defect > reduction/improvements as described above. How about we make the standard, "Would this fix be backported to stable?" > > > Having to redo a patch is already a huge headache. Normally, I already > > considered the reviewer's prefered style and decided I didn't like it. > > Example please. We both seem to prefer consistent style. > For example, if you have a signedness bugs: ret = frob(unsigned_long_size); - if (ret < unsigned_long_size) + if (ret < 0 || ret < unsigned_long_size) vs: + if (ret < (int)unsigned_long_size) goto whatever; To me, whoever fixes the bug gets to choose their prefered style but maybe some reviewers have strong feelings one way or the other. > > Then to make me redo the patch in an ugly style and say thank you on > > top of that??? Forget about it. > > Not a thing I've asked for. > > > Plus, as a reviewer I hate reviewing patches over and over. > > interdiff could be improved. > > > I've argued for years that we should have a Fixes-from: tag. The zero > > day bot is already encouraging people to add Reported-by tags for this > > and a lot of people do. > > It's still a question of what fixes means in any context. > > https://www.google.com/search?q=%27fixes-from%3A%27%20carpenter%20site%3Alore.kernel.org > gives: > It looks like there aren't many great matches for your search > No, I mean people add Reported-by tags for fixes to the original commit like in commit f026d8ca2904 ("igc: add support to eeprom, registers and link self-tests"). regards, dan carpenter _______________________________________________ Ksummit-discuss mailing list Ksummit-discuss@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/ksummit-discuss ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Ksummit-discuss] crediting bug reports and fixes folded into original patch 2020-12-09 10:30 ` Dan Carpenter @ 2020-12-09 17:45 ` Dan Williams 0 siblings, 0 replies; 25+ messages in thread From: Dan Williams @ 2020-12-09 17:45 UTC (permalink / raw) To: Dan Carpenter Cc: ksummit-discuss, Greg KH, LKML, Colin Ian King, Vlastimil Babka On Wed, Dec 9, 2020 at 2:31 AM Dan Carpenter <dan.carpenter@oracle.com> wrote: > > On Wed, Dec 09, 2020 at 12:54:30AM -0800, Joe Perches wrote: > > On Wed, 2020-12-09 at 10:58 +0300, Dan Carpenter wrote: > > > On Tue, Dec 08, 2020 at 09:01:49PM -0800, Joe Perches wrote: > > > > On Tue, 2020-12-08 at 16:34 -0800, Kees Cook wrote: > > > > > > > > > If not "Adjusted-by", what about "Tweaked-by", "Helped-by", > > > > > "Corrected-by"? > > > > > > > > Improved-by: / Enhanced-by: / Revisions-by: > > > > > > > > > > I don't think we should give any credit for improvements or enhancements, > > > only for fixes. > > > > Hey Dan. > > > > I do. If a patch isn't comprehensive and a reviewer notices some > > missing coverage or algorithmic performance enhancement, I think that > > should be noted. > > > > > Complaining about style is its own reward. > > > > <chuckle, maybe so. I view it more like coaching...> > > > > I believe I've said multiple times that style changes shouldn't require > > additional commentary added to a patch. > > > > I'm not making any suggestion to comment for style, only logic or defect > > reduction/improvements as described above. > > How about we make the standard, "Would this fix be backported to stable?" > > > > > > Having to redo a patch is already a huge headache. Normally, I already > > > considered the reviewer's prefered style and decided I didn't like it. > > > > Example please. We both seem to prefer consistent style. > > > > For example, if you have a signedness bugs: > > ret = frob(unsigned_long_size); > - if (ret < unsigned_long_size) > + if (ret < 0 || ret < unsigned_long_size) > vs: > + if (ret < (int)unsigned_long_size) > goto whatever; > > To me, whoever fixes the bug gets to choose their prefered style but > maybe some reviewers have strong feelings one way or the other. > > > > Then to make me redo the patch in an ugly style and say thank you on > > > top of that??? Forget about it. > > > > Not a thing I've asked for. > > > > > Plus, as a reviewer I hate reviewing patches over and over. > > > > interdiff could be improved. > > > > > I've argued for years that we should have a Fixes-from: tag. The zero > > > day bot is already encouraging people to add Reported-by tags for this > > > and a lot of people do. > > > > It's still a question of what fixes means in any context. > > > > https://www.google.com/search?q=%27fixes-from%3A%27%20carpenter%20site%3Alore.kernel.org > > gives: > > It looks like there aren't many great matches for your search > > > > No, I mean people add Reported-by tags for fixes to the original commit > like in commit f026d8ca2904 ("igc: add support to eeprom, registers and > link self-tests"). For after the fact post-processing of tags to generate summary reports, is there a significant difference between Reported-by for "original commit motivation" and Reported-by for "follow-on fixups"? Especially since this practice of Reported-by for fixups is already deployed in the tree (at least kbuild-robot credit reports and my subsystems operate this way). If the fix is a slightly late and needs to come as a follow-on patch the tag will be Reported-by on that fix. I fail to perceive a benefit in augmenting the tag to indicate the resolution of the race condition of the commit making it upstream. _______________________________________________ Ksummit-discuss mailing list Ksummit-discuss@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/ksummit-discuss ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Ksummit-discuss] crediting bug reports and fixes folded into original patch 2020-12-02 23:43 [Ksummit-discuss] crediting bug reports and fixes folded into original patch Vlastimil Babka 2020-12-03 4:02 ` Dan Williams @ 2020-12-03 10:33 ` Dan Carpenter 2020-12-03 13:41 ` Julia Lawall 2020-12-03 13:58 ` James Bottomley 2020-12-04 4:54 ` Theodore Y. Ts'o 3 siblings, 1 reply; 25+ messages in thread From: Dan Carpenter @ 2020-12-03 10:33 UTC (permalink / raw) To: Vlastimil Babka; +Cc: LKML, ksummit-discuss I'd like a "Fixes-from: Name email" tag for when someone spots a bug in a patch. I think we should not give credit for style complaints, because those are their own reward and we already have enough bike shedding. regards, dan carpenter _______________________________________________ Ksummit-discuss mailing list Ksummit-discuss@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/ksummit-discuss ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Ksummit-discuss] crediting bug reports and fixes folded into original patch 2020-12-03 10:33 ` Dan Carpenter @ 2020-12-03 13:41 ` Julia Lawall 0 siblings, 0 replies; 25+ messages in thread From: Julia Lawall @ 2020-12-03 13:41 UTC (permalink / raw) To: Dan Carpenter; +Cc: ksummit-discuss, LKML, Vlastimil Babka On Thu, 3 Dec 2020, Dan Carpenter wrote: > I'd like a "Fixes-from: Name email" tag for when someone spots a bug in > a patch. > > I think we should not give credit for style complaints, because those > are their own reward and we already have enough bike shedding. I agree with Dan, although I'm quite ok with the current situation as well. julia _______________________________________________ Ksummit-discuss mailing list Ksummit-discuss@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/ksummit-discuss ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Ksummit-discuss] crediting bug reports and fixes folded into original patch 2020-12-02 23:43 [Ksummit-discuss] crediting bug reports and fixes folded into original patch Vlastimil Babka 2020-12-03 4:02 ` Dan Williams 2020-12-03 10:33 ` Dan Carpenter @ 2020-12-03 13:58 ` James Bottomley 2020-12-03 16:55 ` Joe Perches 2020-12-03 18:52 ` Matthew Wilcox 2020-12-04 4:54 ` Theodore Y. Ts'o 3 siblings, 2 replies; 25+ messages in thread From: James Bottomley @ 2020-12-03 13:58 UTC (permalink / raw) To: Vlastimil Babka, ksummit-discuss; +Cc: LKML On Thu, 2020-12-03 at 00:43 +0100, Vlastimil Babka wrote: > Hi, > > there was a bit of debate on Twitter about this, so I thought I would > bring it here. Imagine a scenario where patch sits as a commit in > -next and there's a bug report or fix, possibly by a bot or with some > static analysis. The maintainer decides to fold it into the original > patch, which makes sense for e.g. bisectability. But there seem to be > no clear rules about attribution in this case, which looks like there > should be, probably in > Documentation/maintainer/modifying-patches.rst > > The original bug fix might include a From: $author, a Reported-by: > (e.g. syzbot), Fixes: $next-commit, some tag such as Addresses- > Coverity: to credit the static analysis tool, and an SoB. After > folding, all that's left might be a line as "include fix from > $author" in the SoB area. This is a loss of metadata/attribution just > due to folding, and might make contributors unhappy. Had they sent > the fix after the original commit was mainline and immutable, all > the info above would "survive" in the form of new commit. It has been the case since forever that discussion which improves an uncommitted patch is only captured in email (which now may be preserved in a link tag). Patch updates that come in after the patch is committed get their own commit. We've tried to move people away from counting commits as an indicator of upstream eminence, but it's still a fact of life that this is what matters to a lot of open source community managers. The tension we have is between liking a clean commit in the tree as opposed to a sequence of commits tracking the evolution of the patch and this community manager desire to track patches. So there are two embedded questions here: firstly, should we be as wedded to clean history as we are, because showing the evolution would simply solve this? Secondly, if we are agreed on clean history, how can we make engagement via email as important as engagement via commit for the community managers so the Link tag is enough? I've got to say I think trying to add tags to recognize patch evolution is a mistake and we instead investigate one of the two proposals above. James _______________________________________________ Ksummit-discuss mailing list Ksummit-discuss@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/ksummit-discuss ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Ksummit-discuss] crediting bug reports and fixes folded into original patch 2020-12-03 13:58 ` James Bottomley @ 2020-12-03 16:55 ` Joe Perches 2020-12-03 19:17 ` Konstantin Ryabitsev 2020-12-03 18:52 ` Matthew Wilcox 1 sibling, 1 reply; 25+ messages in thread From: Joe Perches @ 2020-12-03 16:55 UTC (permalink / raw) To: James Bottomley, Vlastimil Babka, ksummit-discuss; +Cc: LKML On Thu, 2020-12-03 at 05:58 -0800, James Bottomley wrote: > So there are two embedded questions here: firstly, should we be as > wedded to clean history as we are, because showing the evolution would > simply solve this? Secondly, if we are agreed on clean history, how > can we make engagement via email as important as engagement via commit > for the community managers so the Link tag is enough? I've got to say > I think trying to add tags to recognize patch evolution is a mistake > and we instead investigate one of the two proposals above. I don't care that any trivial style notes I give to anyone are tracked for posterity. Who are these 'community managers' that use these? Signatures are a mechanism for credit tracking isn't great. One style that seems to have been generally accepted is for patch revision change logs to be noted below a --- line. Often that change log will shows various improvements made to a patch and the people and reasoning that helped make those improvements. Perhaps automate a mechanism to capture that information as git notes for the patches when applied. _______________________________________________ Ksummit-discuss mailing list Ksummit-discuss@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/ksummit-discuss ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Ksummit-discuss] crediting bug reports and fixes folded into original patch 2020-12-03 16:55 ` Joe Perches @ 2020-12-03 19:17 ` Konstantin Ryabitsev 2020-12-03 19:24 ` Joe Perches 2020-12-03 21:13 ` James Bottomley 0 siblings, 2 replies; 25+ messages in thread From: Konstantin Ryabitsev @ 2020-12-03 19:17 UTC (permalink / raw) To: Joe Perches; +Cc: James Bottomley, ksummit-discuss, Vlastimil Babka, LKML On Thu, Dec 03, 2020 at 08:55:54AM -0800, Joe Perches wrote: > Perhaps automate a mechanism to capture that information as > git notes for the patches when applied. Git notes have a limited usefulness for this -- they are indeed part of the repository, but they aren't replicated unless someone does a --mirror clone (or specifically fetches refs/notes/*). If the goal is to improve visibility for contributors, then putting this info into a git note will hardly make more difference than providing a Link: that someone has to follow to a list archival service. I can offer the following proposal: - kernel.org already monitors all mailing lists that are archived on lore.kernel.org for the purposes of pull request tracking (pr-tracker-bot). - in the near future, we will add a separate process that will auto-explode all pull requests into individual patches and add them to a separate public-inbox archive (think of it as another transparency log, since pull requests are transient and opaque). We can additionally: - identify all Link: and Message-Id: entries in commit messages, retrieve the threads they refer to, and archive them as part of the same (or adjacent) transparency log. This offers an improvement over the status quo, because if lore.kernel.org becomes unavailable, someone would have to have access to all backend archive repositories it is currently tracking in order to be able to reconstitute relevant conversations -- whereas with this change, it should be sufficient to just have the copy of the transparency log to have a fully self-contained high-relevancy archive of both individual commits and conversations that happened around them. I'm just not sure if this will help with the subject of the conversation, or if it does not serve the goal of recognizing developer contributions by making them more visible. -K _______________________________________________ Ksummit-discuss mailing list Ksummit-discuss@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/ksummit-discuss ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Ksummit-discuss] crediting bug reports and fixes folded into original patch 2020-12-03 19:17 ` Konstantin Ryabitsev @ 2020-12-03 19:24 ` Joe Perches 2020-12-03 21:13 ` James Bottomley 1 sibling, 0 replies; 25+ messages in thread From: Joe Perches @ 2020-12-03 19:24 UTC (permalink / raw) To: Konstantin Ryabitsev Cc: James Bottomley, ksummit-discuss, Vlastimil Babka, LKML On Thu, 2020-12-03 at 14:17 -0500, Konstantin Ryabitsev wrote: > On Thu, Dec 03, 2020 at 08:55:54AM -0800, Joe Perches wrote: > > Perhaps automate a mechanism to capture that information as > > git notes for the patches when applied. > > Git notes have a limited usefulness for this -- they are indeed part of > the repository, but they aren't replicated unless someone does a > --mirror clone (or specifically fetches refs/notes/*). If the goal is to > improve visibility for contributors, then putting this info into a git > note will hardly make more difference than providing a Link: that > someone has to follow to a list archival service. Or it becomes standard to fetch the refs/notes/... at the pull time. > I can offer the following proposal: > > - kernel.org already monitors all mailing lists that are archived on > lore.kernel.org for the purposes of pull request tracking > (pr-tracker-bot). > - in the near future, we will add a separate process that will > auto-explode all pull requests into individual patches and add them > to a separate public-inbox archive (think of it as another > transparency log, since pull requests are transient and opaque). > > We can additionally: > > - identify all Link: and Message-Id: entries in commit messages, > retrieve the threads they refer to, and archive them as part of the > same (or adjacent) transparency log. > > This offers an improvement over the status quo, because if > lore.kernel.org becomes unavailable, someone would have to have access > to all backend archive repositories it is currently tracking in order to > be able to reconstitute relevant conversations -- whereas with this > change, it should be sufficient to just have the copy of the > transparency log to have a fully self-contained high-relevancy archive > of both individual commits and conversations that happened around them. I think that would be great. Thanks. I think all the requests for additional -by: -from: signature/bylines becoe unnecessary if and when this proposal is implemented. _______________________________________________ Ksummit-discuss mailing list Ksummit-discuss@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/ksummit-discuss ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Ksummit-discuss] crediting bug reports and fixes folded into original patch 2020-12-03 19:17 ` Konstantin Ryabitsev 2020-12-03 19:24 ` Joe Perches @ 2020-12-03 21:13 ` James Bottomley 1 sibling, 0 replies; 25+ messages in thread From: James Bottomley @ 2020-12-03 21:13 UTC (permalink / raw) To: Konstantin Ryabitsev, Joe Perches; +Cc: Vlastimil Babka, ksummit-discuss, LKML On Thu, 2020-12-03 at 14:17 -0500, Konstantin Ryabitsev wrote: > On Thu, Dec 03, 2020 at 08:55:54AM -0800, Joe Perches wrote: > > Perhaps automate a mechanism to capture that information as > > git notes for the patches when applied. > > Git notes have a limited usefulness for this -- they are indeed part > of the repository, but they aren't replicated unless someone does a > --mirror clone (or specifically fetches refs/notes/*). If the goal is > to improve visibility for contributors, then putting this info into a > git note will hardly make more difference than providing a Link: that > someone has to follow to a list archival service. > > I can offer the following proposal: > > - kernel.org already monitors all mailing lists that are archived on > lore.kernel.org for the purposes of pull request tracking > (pr-tracker-bot). > - in the near future, we will add a separate process that will > auto-explode all pull requests into individual patches and add them > to a separate public-inbox archive (think of it as another > transparency log, since pull requests are transient and opaque). > > We can additionally: > > - identify all Link: and Message-Id: entries in commit messages, > retrieve the threads they refer to, and archive them as part of > the same (or adjacent) transparency log. > > This offers an improvement over the status quo, because if > lore.kernel.org becomes unavailable, someone would have to have > access to all backend archive repositories it is currently tracking > in order to be able to reconstitute relevant conversations -- whereas > with this change, it should be sufficient to just have the copy of > the transparency log to have a fully self-contained high-relevancy > archive of both individual commits and conversations that happened > around them. I don't think this is strictly necessary because there's more than lore that archive's our lists, but the people at the internet history project would remind me not to look a gift horse in the mouth, so I think this would certainly be a useful addition. The thing which Link: doesn't necessarily track is iterations, so if you replied to v2 and your feedback got incorporated, there's a v3 iteration which has a different msgid. Is there a way of getting this full history, not just the current thread? > I'm just not sure if this will help with the subject of the > conversation, or if it does not serve the goal of recognizing > developer contributions by making them more visible. I added Jon to the cc since a lot of managers (community or otherwise) do use the lwn.net stats as a performance guide. The real question is could we get something measurable out of the data? say number of replies to an accepted patch counting in the reviewer stats or something? James _______________________________________________ Ksummit-discuss mailing list Ksummit-discuss@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/ksummit-discuss ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Ksummit-discuss] crediting bug reports and fixes folded into original patch 2020-12-03 13:58 ` James Bottomley 2020-12-03 16:55 ` Joe Perches @ 2020-12-03 18:52 ` Matthew Wilcox 2020-12-03 20:04 ` James Bottomley 1 sibling, 1 reply; 25+ messages in thread From: Matthew Wilcox @ 2020-12-03 18:52 UTC (permalink / raw) To: James Bottomley; +Cc: ksummit, Vlastimil Babka, LKML [-- Attachment #1.1: Type: text/plain, Size: 2680 bytes --] It's not so much "clean history" that's the desire. It's "don't leave landmines for git bisect". On Thu., Dec. 3, 2020, 08:58 James Bottomley, < James.Bottomley@hansenpartnership.com> wrote: > On Thu, 2020-12-03 at 00:43 +0100, Vlastimil Babka wrote: > > Hi, > > > > there was a bit of debate on Twitter about this, so I thought I would > > bring it here. Imagine a scenario where patch sits as a commit in > > -next and there's a bug report or fix, possibly by a bot or with some > > static analysis. The maintainer decides to fold it into the original > > patch, which makes sense for e.g. bisectability. But there seem to be > > no clear rules about attribution in this case, which looks like there > > should be, probably in > > Documentation/maintainer/modifying-patches.rst > > > > The original bug fix might include a From: $author, a Reported-by: > > (e.g. syzbot), Fixes: $next-commit, some tag such as Addresses- > > Coverity: to credit the static analysis tool, and an SoB. After > > folding, all that's left might be a line as "include fix from > > $author" in the SoB area. This is a loss of metadata/attribution just > > due to folding, and might make contributors unhappy. Had they sent > > the fix after the original commit was mainline and immutable, all > > the info above would "survive" in the form of new commit. > > It has been the case since forever that discussion which improves an > uncommitted patch is only captured in email (which now may be preserved > in a link tag). Patch updates that come in after the patch is > committed get their own commit. We've tried to move people away from > counting commits as an indicator of upstream eminence, but it's still a > fact of life that this is what matters to a lot of open source > community managers. The tension we have is between liking a clean > commit in the tree as opposed to a sequence of commits tracking the > evolution of the patch and this community manager desire to track > patches. > > So there are two embedded questions here: firstly, should we be as > wedded to clean history as we are, because showing the evolution would > simply solve this? Secondly, if we are agreed on clean history, how > can we make engagement via email as important as engagement via commit > for the community managers so the Link tag is enough? I've got to say > I think trying to add tags to recognize patch evolution is a mistake > and we instead investigate one of the two proposals above. > > James > > > _______________________________________________ > Ksummit-discuss mailing list > Ksummit-discuss@lists.linuxfoundation.org > https://lists.linuxfoundation.org/mailman/listinfo/ksummit-discuss > [-- Attachment #1.2: Type: text/html, Size: 3463 bytes --] [-- Attachment #2: Type: text/plain, Size: 186 bytes --] _______________________________________________ Ksummit-discuss mailing list Ksummit-discuss@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/ksummit-discuss ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Ksummit-discuss] crediting bug reports and fixes folded into original patch 2020-12-03 18:52 ` Matthew Wilcox @ 2020-12-03 20:04 ` James Bottomley 0 siblings, 0 replies; 25+ messages in thread From: James Bottomley @ 2020-12-03 20:04 UTC (permalink / raw) To: Matthew Wilcox; +Cc: Vlastimil Babka, ksummit, LKML On Thu, 2020-12-03 at 13:52 -0500, Matthew Wilcox wrote: > It's not so much "clean history" that's the desire. It's "don't leave > landmines for git bisect". ... top posting? Well functional git bisect and show the evolution of the patch aren't mutually exclusive. Plus our current clean history approach doesn't always detect them ... That said, I agree that if a review comes in that shows a patch would break the build or runtime in a way that would damage bisection, it should be folded. I suppose this argues that only less trivial changes can be shown as part of the unfolded history, and since they're obviously less important than the review driven folded change it would add more bias to track them. I fall back to my link is enough comment. James _______________________________________________ Ksummit-discuss mailing list Ksummit-discuss@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/ksummit-discuss ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Ksummit-discuss] crediting bug reports and fixes folded into original patch 2020-12-02 23:43 [Ksummit-discuss] crediting bug reports and fixes folded into original patch Vlastimil Babka ` (2 preceding siblings ...) 2020-12-03 13:58 ` James Bottomley @ 2020-12-04 4:54 ` Theodore Y. Ts'o 3 siblings, 0 replies; 25+ messages in thread From: Theodore Y. Ts'o @ 2020-12-04 4:54 UTC (permalink / raw) To: Vlastimil Babka; +Cc: LKML, ksummit-discuss On Thu, Dec 03, 2020 at 12:43:52AM +0100, Vlastimil Babka wrote: > > there was a bit of debate on Twitter about this, so I thought I would bring it > here. Imagine a scenario where patch sits as a commit in -next and there's a bug > report or fix, possibly by a bot or with some static analysis. The maintainer > decides to fold it into the original patch, which makes sense for e.g. > bisectability. But there seem to be no clear rules about attribution in this > case, which looks like there should be, probably in > Documentation/maintainer/modifying-patches.rst I don't think there should be any kind of fixed, inflexible rules about this. 1) Sometimes there will be a *huge* number of comments and suggestions. Do we really want to require links to dozens of mail message id's, and/or dozens or more e-mail addresses? 2) Sometimes a fixup is pretty trivial; even if it is expressed in the form of a one-line patch, versus someone who does a detailed review of a patch, but doesn't actually end up appending an explicit Reviewed-by, perhaps because he or she didn't completely agree with the final version of the patch. 3) I think this very much should be up to the maintainer's discretion, as opposed to making rules that may result in some rediculous amount of bloat in the git log. 4) It's really unhealthy, in my opinion for people to be fixed on counting attributions. If we create fixed rules, this can turn into people try to game the system. It's the same reason why I'm not terribly enthusiastic about people trying to game Signed-off-by counts by sending gazillions of white space or spelling fixes. If the fix is large enough that for copyright reasons we need to acknowledge the work, then folding in the SoB as for DCO reason makes perfect sense. But if it's a trivial patch (the kind where projects that require copyright assignment wouldn't require executed legal agreements), then perhaps attribution is not always a requirement. Again, there are times when people who spend a lot of work discussing patch may not get attributiionm even if they didn't actually create the one-line whitespace fix and sent it in as a patch with a signed-off-by with a demand that the attribution be preserved. Common sense really needs to prevale here, and I'm concerned that people who like to create rules don't realize what a mess this can create when contributors approach their participation with a sense of entitlement. Cheers, - Ted _______________________________________________ Ksummit-discuss mailing list Ksummit-discuss@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/ksummit-discuss ^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2020-12-09 17:45 UTC | newest] Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-12-02 23:43 [Ksummit-discuss] crediting bug reports and fixes folded into original patch Vlastimil Babka 2020-12-03 4:02 ` Dan Williams 2020-12-03 9:34 ` Leon Romanovsky 2020-12-03 9:36 ` Geert Uytterhoeven 2020-12-03 10:40 ` Leon Romanovsky 2020-12-03 18:30 ` Greg KH 2020-12-03 19:04 ` Leon Romanovsky 2020-12-09 0:34 ` Kees Cook 2020-12-09 5:01 ` Joe Perches 2020-12-09 7:58 ` Dan Carpenter 2020-12-09 8:45 ` Vlastimil Babka 2020-12-09 9:18 ` Geert Uytterhoeven 2020-12-09 8:54 ` Joe Perches 2020-12-09 10:30 ` Dan Carpenter 2020-12-09 17:45 ` Dan Williams 2020-12-03 10:33 ` Dan Carpenter 2020-12-03 13:41 ` Julia Lawall 2020-12-03 13:58 ` James Bottomley 2020-12-03 16:55 ` Joe Perches 2020-12-03 19:17 ` Konstantin Ryabitsev 2020-12-03 19:24 ` Joe Perches 2020-12-03 21:13 ` James Bottomley 2020-12-03 18:52 ` Matthew Wilcox 2020-12-03 20:04 ` James Bottomley 2020-12-04 4:54 ` Theodore Y. Ts'o
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).