2019-11-15 15:05:53

by Eugeniu Rosca

[permalink] [raw]
Subject: [PATCH] checkpatch: whitelist Originally-by: signature

Oftentimes [1], the contributor would like to honor or give credits [2]
to somebody's original ideas in the submission/reviewing process. While
"Co-developed-by:" and "Suggested-by:" (currently whitelisted) could be
employed for this purpose, they are not ideal.

Below matrix attempts portraying/quantifying the subtle differences
between these signatures (subjective/oversimplified):

Helper signature Contribution "ownership"
None 100% Author
Suggested-by: X 80% Author / 20% "X"
Co-developed-by: X 50% Author / 50% "X"
Originally-by: X 20% Author / 80% "X"

[1] linux (v5.4-rc7) git log --oneline --grep Originally-by | wc -l
88
[2] https://lore.kernel.org/lkml/[email protected]/

Cc: Thomas Gleixner <[email protected]>
Signed-off-by: Eugeniu Rosca <[email protected]>
---
scripts/checkpatch.pl | 1 +
1 file changed, 1 insertion(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 6fcc66afb088..e456aba12bd0 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -486,6 +486,7 @@ our $signature_tags = qr{(?xi:
Reviewed-by:|
Reported-by:|
Suggested-by:|
+ Originally-by:|
To:|
Cc:
)};
--
2.24.0


2019-11-15 15:12:47

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] checkpatch: whitelist Originally-by: signature

On Fri, 2019-11-15 at 16:02 +0100, Eugeniu Rosca wrote:
> Oftentimes [1], the contributor would like to honor or give credits [2]
> to somebody's original ideas in the submission/reviewing process. While
> "Co-developed-by:" and "Suggested-by:" (currently whitelisted) could be
> employed for this purpose, they are not ideal.

You need to get the use of this accepted into Documentation/process
before adding it to checkpatch


2019-11-15 15:49:22

by Eugeniu Rosca

[permalink] [raw]
Subject: Re: [PATCH] checkpatch: whitelist Originally-by: signature

Hi Jonathan,

On Fri, Nov 15, 2019 at 07:09:17AM -0800, Joe Perches wrote:
> On Fri, 2019-11-15 at 16:02 +0100, Eugeniu Rosca wrote:
> > Oftentimes [1], the contributor would like to honor or give credits [2]
> > to somebody's original ideas in the submission/reviewing process. While
> > "Co-developed-by:" and "Suggested-by:" (currently whitelisted) could be
> > employed for this purpose, they are not ideal.
>
> You need to get the use of this accepted into Documentation/process
> before adding it to checkpatch

If the change [*] makes sense to you, I can submit an update to
Documentation/process/submitting-patches.rst

[*] https://marc.info/?l=linux-kernel&m=157383014408527&w=2

--
Best Regards,
Eugeniu

2019-11-15 16:31:44

by Jonathan Corbet

[permalink] [raw]
Subject: Re: [PATCH] checkpatch: whitelist Originally-by: signature

On Fri, 15 Nov 2019 16:46:27 +0100
Eugeniu Rosca <[email protected]> wrote:

> On Fri, Nov 15, 2019 at 07:09:17AM -0800, Joe Perches wrote:
> > On Fri, 2019-11-15 at 16:02 +0100, Eugeniu Rosca wrote:
> > > Oftentimes [1], the contributor would like to honor or give credits [2]
> > > to somebody's original ideas in the submission/reviewing process. While
> > > "Co-developed-by:" and "Suggested-by:" (currently whitelisted) could be
> > > employed for this purpose, they are not ideal.
> >
> > You need to get the use of this accepted into Documentation/process
> > before adding it to checkpatch
>
> If the change [*] makes sense to you, I can submit an update to
> Documentation/process/submitting-patches.rst

So there appear to be 89 patches with Originally-by in the entire Git
history, which isn't a a lot; there are 3x as many Co-developed-by tags,
which still isn't a huge number. I do wonder if it's worth recognizing
yet another tag with a subtly different shade of meaning here? My own
opinion doesn't matter a lot, but I'd like to have a sense that there is
wider acceptance of this tag before adding it to the docs.

Thanks,

jon

2019-11-15 17:25:54

by Eugeniu Rosca

[permalink] [raw]
Subject: Re: [PATCH] checkpatch: whitelist Originally-by: signature

Hi Jonathan,

On Fri, Nov 15, 2019 at 09:29:43AM -0700, Jonathan Corbet wrote:
> On Fri, 15 Nov 2019 16:46:27 +0100
> Eugeniu Rosca <[email protected]> wrote:
>
> > On Fri, Nov 15, 2019 at 07:09:17AM -0800, Joe Perches wrote:
> > > On Fri, 2019-11-15 at 16:02 +0100, Eugeniu Rosca wrote:
> > > > Oftentimes [1], the contributor would like to honor or give credits [2]
> > > > to somebody's original ideas in the submission/reviewing process. While
> > > > "Co-developed-by:" and "Suggested-by:" (currently whitelisted) could be
> > > > employed for this purpose, they are not ideal.
> > >
> > > You need to get the use of this accepted into Documentation/process
> > > before adding it to checkpatch
> >
> > If the change [*] makes sense to you, I can submit an update to
> > Documentation/process/submitting-patches.rst
>
> So there appear to be 89 patches with Originally-by in the entire Git
> history, which isn't a a lot; there are 3x as many Co-developed-by tags,
> which still isn't a huge number. I do wonder if it's worth recognizing
> yet another tag with a subtly different shade of meaning here? My own
> opinion doesn't matter a lot, but I'd like to have a sense that there is
> wider acceptance of this tag before adding it to the docs.

I will give a real-life example. Say, I have some patches in my
local tree and they've been developed by somebody who is no longer
interested/paid to upstream those.

I first submit those patches with the original authorship, plus my SoB.
Then, the reviewers post their findings. I put my time into fixing those
and re-testing the patch or the entire series. The final patch/series
may look totally different compared to the original one.

Which way would you suggest to give credits to the original author?
I personally think that "Co-developed-by:" conveys the idea/feeling of
"teaming up" with somebody, which doesn't happen in my example.

--
Best Regards,
Eugeniu

2019-11-15 21:35:52

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] checkpatch: whitelist Originally-by: signature

On Fri, 2019-11-15 at 09:29 -0700, Jonathan Corbet wrote:
> On Fri, 15 Nov 2019 16:46:27 +0100
> Eugeniu Rosca <[email protected]> wrote:
>
> > On Fri, Nov 15, 2019 at 07:09:17AM -0800, Joe Perches wrote:
> > > On Fri, 2019-11-15 at 16:02 +0100, Eugeniu Rosca wrote:
> > > > Oftentimes [1], the contributor would like to honor or give credits [2]
> > > > to somebody's original ideas in the submission/reviewing process. While
> > > > "Co-developed-by:" and "Suggested-by:" (currently whitelisted) could be
> > > > employed for this purpose, they are not ideal.
> > >
> > > You need to get the use of this accepted into Documentation/process
> > > before adding it to checkpatch
> >
> > If the change [*] makes sense to you, I can submit an update to
> > Documentation/process/submitting-patches.rst
>
> So there appear to be 89 patches with Originally-by in the entire Git
> history, which isn't a a lot; there are 3x as many Co-developed-by tags,
> which still isn't a huge number. I do wonder if it's worth recognizing
> yet another tag with a subtly different shade of meaning here? My own
> opinion doesn't matter a lot, but I'd like to have a sense that there is
> wider acceptance of this tag before adding it to the docs.

I am also not a proponent of adding this as a new tag/signature.


2019-11-27 09:28:14

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH] checkpatch: whitelist Originally-by: signature

Hi Eugeniu,

On Fri, Nov 15, 2019 at 6:24 PM Eugeniu Rosca <[email protected]> wrote:
> On Fri, Nov 15, 2019 at 09:29:43AM -0700, Jonathan Corbet wrote:
> > On Fri, 15 Nov 2019 16:46:27 +0100
> > Eugeniu Rosca <[email protected]> wrote:
> > > On Fri, Nov 15, 2019 at 07:09:17AM -0800, Joe Perches wrote:
> > > > On Fri, 2019-11-15 at 16:02 +0100, Eugeniu Rosca wrote:
> > > > > Oftentimes [1], the contributor would like to honor or give credits [2]
> > > > > to somebody's original ideas in the submission/reviewing process. While
> > > > > "Co-developed-by:" and "Suggested-by:" (currently whitelisted) could be
> > > > > employed for this purpose, they are not ideal.
> > > >
> > > > You need to get the use of this accepted into Documentation/process
> > > > before adding it to checkpatch
> > >
> > > If the change [*] makes sense to you, I can submit an update to
> > > Documentation/process/submitting-patches.rst
> >
> > So there appear to be 89 patches with Originally-by in the entire Git
> > history, which isn't a a lot; there are 3x as many Co-developed-by tags,
> > which still isn't a huge number. I do wonder if it's worth recognizing
> > yet another tag with a subtly different shade of meaning here? My own
> > opinion doesn't matter a lot, but I'd like to have a sense that there is
> > wider acceptance of this tag before adding it to the docs.
>
> I will give a real-life example. Say, I have some patches in my
> local tree and they've been developed by somebody who is no longer
> interested/paid to upstream those.
>
> I first submit those patches with the original authorship, plus my SoB.
> Then, the reviewers post their findings. I put my time into fixing those
> and re-testing the patch or the entire series. The final patch/series
> may look totally different compared to the original one.
>
> Which way would you suggest to give credits to the original author?
> I personally think that "Co-developed-by:" conveys the idea/feeling of
> "teaming up" with somebody, which doesn't happen in my example.

What I typically do is this:
1. If the changes due to review are minor, I just add my SoB below the
original SoB,
2. If the changes are not insignificant, I also add a line "[geert: Did foo]"
in between the original SoB and mine,
3. If the patch needed a complete rewrite, I assume ownership, and add
"Based on/inspired by ..." to the patch description to give credit.

Hope this helps (and is acceptable for other people ;-)

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2019-11-27 11:27:18

by Eugeniu Rosca

[permalink] [raw]
Subject: Re: [PATCH] checkpatch: whitelist Originally-by: signature

Hi Geert,

On Wed, Nov 27, 2019 at 10:25:29AM +0100, Geert Uytterhoeven wrote:
[..]
> On Fri, Nov 15, 2019 at 6:24 PM Eugeniu Rosca <[email protected]> wrote:
[..]
> > I will give a real-life example. Say, I have some patches in my
> > local tree and they've been developed by somebody who is no longer
> > interested/paid to upstream those.
> >
> > I first submit those patches with the original authorship, plus my SoB.
> > Then, the reviewers post their findings. I put my time into fixing those
> > and re-testing the patch or the entire series. The final patch/series
> > may look totally different compared to the original one.
> >
> > Which way would you suggest to give credits to the original author?
> > I personally think that "Co-developed-by:" conveys the idea/feeling of
> > "teaming up" with somebody, which doesn't happen in my example.
>
> What I typically do is this:
> 1. If the changes due to review are minor, I just add my SoB below the
> original SoB,
> 2. If the changes are not insignificant, I also add a line "[geert: Did foo]"
> in between the original SoB and mine,
> 3. If the patch needed a complete rewrite, I assume ownership, and add
> "Based on/inspired by ..." to the patch description to give credit.
>
> Hope this helps (and is acceptable for other people ;-)

Thank you for your time to share the best practices from the heart of
Linux kernel community. This looks like a reasonable blueprint to follow
and I will personally bookmark and quote it whenever appropriate.

The way I see "Originally-by" is that it attempts to replace the free
wording implied at #3 (i.e. patch rewrite case) and hence its benefit.

The less words I have to creatively write by myself, the less errors
I'll make, the less ambiguous my patch will be, the more time I'll have
to dedicate to the important parts of the patch description (feature
overview, bug reproduction, test scenario, etc).

I also understand the desire not to make the process more complicated
than it needs to be. I expect the signature to still pop up here and
there and whether it makes sense to whitelist it, time will tell.

--
Best Regards,
Eugeniu