Re: [Linux-kernel-mentees] Fix for BAD_SIGN_OFF: non-standard signature

[Lukas Bulwahn <lukas.bulwahn@gmail.com>]

> >> Goal 1: Try to map all the non-default signatures to their "standard"
> >> counterpart as much as possible.
> >>
> >> Goal 2: Introduce a few very little signatures to handle those cases
> >> that really cannot be mapped to a non-default signature.
> >>
> >> Provide good rationales that you can defend and provide documentation
> >> for when checkpatch shall explain the fix it proposes.
> >>
> >> Here an example for the first ten cases:
> >>
> >> 1)Debugged-by: 61 -> Codeveloped-by:
> >>
> >> Rationale: Debugging is part of Software Development; so
> >> Codeveloped-by is perfectly fine, even if the contributor did not
> >> create code.
> >>
> >> (alternatively: maybe a new Assisted-by would do here.)
> >>
> >> 2)Requested-by: 48 -> Suggested-by:
> >>
> >> Rationale: In an open-source project, there are "no requests", just
> >> "suggestions" to convince a maintainer to accept your patch.
> >>
> >> 3)Co-authored-by: 43 -> Codeveloped-by:
> >>
> >> Rationale: clear. Codeveloped-by and Co-authored-by are synonyms.
> >>
> >> 4)Originally-by: 39
> >>
> >> Maybe something like this deserves to be a new tag. There is a
> >> significant difference to codeveloped-by. But that needs discussion.
> >>
> >> 5)Analyzed-by: 22
> >>
> >> Rationale: Analyzing is part of Software Development; so
> >> Codeveloped-by is perfectly fine, even if the contributor did not
> >> create code.
> >> (alternatively: maybe a new Assisted-by would do here.)
> >>
> >> 6)Bisected-by: 20
> >>
> >> Difficult...
> >> (maybe a new Assisted-by would do here.)
> >>
> >> 7)Improvements-by: 19 -> Codeveloped-by:
> >>
> >> 8)Generated-by: 17 -> Reported-by: ?
> >>
> >> What does generated-by actually mean?
> >>
> >> 9)Noticed-by: 11 -> Reported-by:
> >>
> >> 10)Inspired-by: 11 -> Suggested-by:
> >>
> >> Maybe you can come up with a list for the next twenty and then we
> >> discuss them with Joe Perches and then a larger group?
> >>
>
> This is the list for next 20:
>
> 11)Original-patch-by: 11 -> co-developed-by / Originally-by (a new
> signoff)
> Rationale: I checked mailing list for one of these signoffs.
> Link1:
> https://lore.kernel.org/linux-perf-users/20190221122306.1511-1-jonas.rabenstein@studium.uni-erlangen.de/
> Link2:
> https://lore.kernel.org/linux-perf-users/20190307174433.28819-32-acme@kernel.org/
>
> Here it seems like someone who started working on the patch but
> couldn't complete it, but still has
> significant contribution in the patch.
> Maybe signing off as codeveloper suffices the purpose. I'm not sure though
>

Agree, that is up for discussion. Either co-developed-by or one new tag.

> 12)Diagnosed-by: 11 -> Maybe 'Reviewed-by' or 'Acked-by'
> Rationale: Observed a few mailing lists, eg here:
> https://lore.kernel.org/lkml/20190609164128.000227333@linuxfoundation.org/
> But could not decide as the user is not adding it along the mails, but
> seems like a maintainer.
>

I do not think Acked-by, maybe co-developed-by or reviewed-by.

> 13)Based-on-a-patch-by: 8 -> Similar to 'Originally-by'
>
> 14)Verified-by: 8 -> Tested-by
> Rationale: Used by a single user. On reading, mailing list, it seems
> that 'Tested-by' tag might be a suitable alternative.
> Link:
> https://lore.kernel.org/lkml/CA+jURcugFhSt9GGRZELQUCnupOf2Ns96Ao5ZruWfVtq=z_7ytw@mail.gmail.com/
>

Agree.

> 15)Okay-ished-by: 8 -> Acked-by
> Rationale: Used by a single user. On reading, mailing list, it seems
> that 'Acked-by' tag might be a suitable alternative.
> Link:
> https://lore.kernel.org/lkml/f06e74e9a38b83ec273196bce727295b828c5870.1507769413.git.rgb@redhat.com/
>

Agree.

> 16)Based-on-patch-by: 7 -> Similar to (13) Based-on-a-patch-by
>

Agree.

> 17)Root-caused-by: 6 -> Maybe 'Fixes:' followed by the commit it is
> fixing.
> Rationale: Going through mailing list, it comes up added with the
> patch. So I couldn't be sure
>

Hmm... you need to show me the cases where this tag is used.

If the tag is not followed by an identity (name + email), it should
not be a signature tag anyway.

> 18)Original-by: 6 -> Similar to '(4)Originally-by'
>

Agree.

> 19)Acked-for-MFD-by: 6 -> Acked-by:
>

Agree.

> 20)Reviewed-off-by: 5 -> Reviewed-by:
>

Agree.

> 21)Based-on-patches-by: 5 -> Similar to (13)
>

Agree.

> 22)Analysed-by: 5 -> Co-developed-by/Reviewed-by
> Rationale: Similar to '(5)Analyzed-by'
>

Agree.

> 23)Based-on-work-by: 5 -> Not sure. Maybe 'Suggested-by'
>

Or similar to 13?

> 24)Proposed-by: 5 -> Maybe 'Suggested-by'
> Rationale: The tag comes up added with the patch,and the user is also
> given the tag 'Signed-off-by', but does not seem to participate in the
> conversation.
> Maybe he is a maintainer, who suggested the patch.
> mailing list:
> https://lore.kernel.org/linux-nvme/20200501212545.21856-3-sagi@grimberg.me/
>

Agree.

> 25)Reported-and-bisected-by: 4 -> Two different tags: 'Reported-by:'
> and 'Bisected-by'
>

Agree.

> 26)Fixed-by: 3 -> Co-developed-by
> Rationale: I observed one of these commit conservations here:
> https://lore.kernel.org/lkml/1b45ffd1-99bb-4ac1-fb65-0de3e42c1c0a@amd.com/
> It seems like there was some bug with this patch, which was fixed by
> the user. I guess Co-developed-by should go well as alternative.
>

Agree.

> 27)Pointed-out-by: 3 -> Suggested-by
> Rationale: For commit 87bd4c26a6c8 ("clocksource/drivers/tegra: Lower
> clocksource rating for some Tegra's"), this warning occurs, where
> the patch is also 'Acked-by' Peter De Schrijver. So, it seems like he
> is a maintainer who must have suggested these changes
>

Agree.

> 28)Suggestions-by: 3 -> Suggested-by
>

Agree.

> 29)Celebrated-by: 3 -> Might be suggested to remove
> Rationale: This tag is used for a single commit 3 times, seems like a
> tag used for celebration of a particular patch
> Link:
> https://lore.kernel.org/lkml/CANRm+CyonYOzGdXo+D8gr8n04=f=S92QH-HxETKnoGGxhMFREA@mail.gmail.com/
>

Agree. Let us suggest deleting such tags.

> 30)Pointed-at-by: 2 -> Suggested-by
> Rationale: One of these tags is named for Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>, who is probably a maintainer.
> Here, the user might just want to acknowledge him for his suggestion,
> so 'Suggested-by' seems appropriate.
>

Agree.

> What do you think?
>

Can you start to implement a patch that creates the basic logic to let
checkpatch.pl suggest the alternatives for those 30 cases above.

Also, it might be good if checkpatch.pl also provides the explanation
why that alternative is proposed (when it is not totally obvious).

Can you also summarize which of the 30 cases need a further discussion with Joe?

Lukas
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees