linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* crediting bug reports and fixes folded into original patch
@ 2020-12-02 23:43 Vlastimil Babka
  2020-12-03  4:02 ` [Ksummit-discuss] " Dan Williams
                   ` (3 more replies)
  0 siblings, 4 replies; 24+ 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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Ksummit-discuss] crediting bug reports and fixes folded into original patch
  2020-12-02 23:43 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; 24+ messages in thread
From: Dan Williams @ 2020-12-03  4:02 UTC (permalink / raw)
  To: Vlastimil Babka; +Cc: ksummit-discuss, LKML

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.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Ksummit-discuss] crediting bug reports and fixes folded into original patch
  2020-12-03  4:02 ` [Ksummit-discuss] " Dan Williams
@ 2020-12-03  9:34   ` Leon Romanovsky
  2020-12-03  9:36     ` Geert Uytterhoeven
  0 siblings, 1 reply; 24+ messages in thread
From: Leon Romanovsky @ 2020-12-03  9:34 UTC (permalink / raw)
  To: Dan Williams; +Cc: Vlastimil Babka, LKML, ksummit-discuss

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

^ permalink raw reply	[flat|nested] 24+ 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; 24+ messages in thread
From: Geert Uytterhoeven @ 2020-12-03  9:36 UTC (permalink / raw)
  To: Leon Romanovsky; +Cc: Dan Williams, ksummit-discuss, LKML, Vlastimil Babka

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Ksummit-discuss] crediting bug reports and fixes folded into original patch
  2020-12-02 23:43 crediting bug reports and fixes folded into original patch Vlastimil Babka
  2020-12-03  4:02 ` [Ksummit-discuss] " 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; 24+ messages in thread
From: Dan Carpenter @ 2020-12-03 10:33 UTC (permalink / raw)
  To: Vlastimil Babka; +Cc: ksummit-discuss, LKML

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


^ permalink raw reply	[flat|nested] 24+ 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; 24+ messages in thread
From: Leon Romanovsky @ 2020-12-03 10:40 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Dan Williams, ksummit-discuss, LKML, Vlastimil Babka

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

^ permalink raw reply	[flat|nested] 24+ 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; 24+ messages in thread
From: Julia Lawall @ 2020-12-03 13:41 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: Vlastimil Babka, LKML, ksummit-discuss



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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Ksummit-discuss] crediting bug reports and fixes folded into original patch
  2020-12-02 23:43 crediting bug reports and fixes folded into original patch Vlastimil Babka
  2020-12-03  4:02 ` [Ksummit-discuss] " Dan Williams
  2020-12-03 10:33 ` Dan Carpenter
@ 2020-12-03 13:58 ` James Bottomley
  2020-12-03 16:55   ` Joe Perches
       [not found]   ` <CAFhKne9ZSbwrH6-g7og2BBEEDGd6ScDnZTNg3znQLvLDCDfeoA@mail.gmail.com>
  2020-12-04  4:54 ` Theodore Y. Ts'o
  3 siblings, 2 replies; 24+ 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



^ permalink raw reply	[flat|nested] 24+ 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
       [not found]   ` <CAFhKne9ZSbwrH6-g7og2BBEEDGd6ScDnZTNg3znQLvLDCDfeoA@mail.gmail.com>
  1 sibling, 1 reply; 24+ 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.



^ permalink raw reply	[flat|nested] 24+ 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; 24+ messages in thread
From: Greg KH @ 2020-12-03 18:30 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Geert Uytterhoeven, Vlastimil Babka, LKML, ksummit-discuss

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

^ permalink raw reply	[flat|nested] 24+ 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; 24+ messages in thread
From: Leon Romanovsky @ 2020-12-03 19:04 UTC (permalink / raw)
  To: Greg KH; +Cc: Geert Uytterhoeven, Vlastimil Babka, LKML, ksummit-discuss

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

^ permalink raw reply	[flat|nested] 24+ 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; 24+ messages in thread
From: Konstantin Ryabitsev @ 2020-12-03 19:17 UTC (permalink / raw)
  To: Joe Perches; +Cc: James Bottomley, Vlastimil Babka, ksummit-discuss, 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

^ permalink raw reply	[flat|nested] 24+ 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; 24+ messages in thread
From: Joe Perches @ 2020-12-03 19:24 UTC (permalink / raw)
  To: Konstantin Ryabitsev
  Cc: James Bottomley, 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.

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.



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Ksummit-discuss] crediting bug reports and fixes folded into original patch
       [not found]   ` <CAFhKne9ZSbwrH6-g7og2BBEEDGd6ScDnZTNg3znQLvLDCDfeoA@mail.gmail.com>
@ 2020-12-03 20:04     ` James Bottomley
  0 siblings, 0 replies; 24+ messages in thread
From: James Bottomley @ 2020-12-03 20:04 UTC (permalink / raw)
  To: Matthew Wilcox; +Cc: ksummit, Vlastimil Babka, 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



^ permalink raw reply	[flat|nested] 24+ 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; 24+ messages in thread
From: James Bottomley @ 2020-12-03 21:13 UTC (permalink / raw)
  To: Konstantin Ryabitsev, Joe Perches
  Cc: ksummit-discuss, Vlastimil Babka, LKML, Jonathan Corbet

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



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [Ksummit-discuss] crediting bug reports and fixes folded into original patch
  2020-12-02 23:43 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; 24+ messages in thread
From: Theodore Y. Ts'o @ 2020-12-04  4:54 UTC (permalink / raw)
  To: Vlastimil Babka; +Cc: ksummit-discuss, LKML

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

^ permalink raw reply	[flat|nested] 24+ 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; 24+ messages in thread
From: Kees Cook @ 2020-12-09  0:34 UTC (permalink / raw)
  To: Greg KH
  Cc: Leon Romanovsky, ksummit-discuss, LKML, Vlastimil Babka,
	Colin Ian King, Dan Carpenter

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

^ permalink raw reply	[flat|nested] 24+ 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; 24+ messages in thread
From: Joe Perches @ 2020-12-09  5:01 UTC (permalink / raw)
  To: Kees Cook, Greg KH
  Cc: Leon Romanovsky, ksummit-discuss, LKML, Vlastimil Babka,
	Colin Ian King, Dan Carpenter

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.




^ permalink raw reply	[flat|nested] 24+ 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; 24+ messages in thread
From: Dan Carpenter @ 2020-12-09  7:58 UTC (permalink / raw)
  To: Joe Perches
  Cc: Kees Cook, Greg KH, ksummit-discuss, 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


^ permalink raw reply	[flat|nested] 24+ 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; 24+ messages in thread
From: Vlastimil Babka @ 2020-12-09  8:45 UTC (permalink / raw)
  To: Dan Carpenter, Joe Perches
  Cc: Kees Cook, Greg KH, ksummit-discuss, LKML, Colin Ian King

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
> 


^ permalink raw reply	[flat|nested] 24+ 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; 24+ messages in thread
From: Joe Perches @ 2020-12-09  8:54 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Kees Cook, Greg KH, ksummit-discuss, 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.



^ permalink raw reply	[flat|nested] 24+ 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; 24+ messages in thread
From: Geert Uytterhoeven @ 2020-12-09  9:18 UTC (permalink / raw)
  To: Vlastimil Babka
  Cc: Dan Carpenter, Joe Perches, Greg KH, Colin Ian King,
	ksummit-discuss, LKML

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

^ permalink raw reply	[flat|nested] 24+ 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; 24+ messages in thread
From: Dan Carpenter @ 2020-12-09 10:30 UTC (permalink / raw)
  To: Joe Perches
  Cc: ksummit-discuss, Greg KH, LKML, Colin Ian King, Vlastimil Babka

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


^ permalink raw reply	[flat|nested] 24+ 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; 24+ messages in thread
From: Dan Williams @ 2020-12-09 17:45 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Joe Perches, Vlastimil Babka, Greg KH, LKML, ksummit-discuss,
	Colin Ian King

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.

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2020-12-09 17:46 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-02 23:43 crediting bug reports and fixes folded into original patch Vlastimil Babka
2020-12-03  4:02 ` [Ksummit-discuss] " 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
     [not found]   ` <CAFhKne9ZSbwrH6-g7og2BBEEDGd6ScDnZTNg3znQLvLDCDfeoA@mail.gmail.com>
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).