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

Aditya yashsri421 at gmail.com
Thu Nov 19 14:09:32 UTC 2020


On 19/11/20 11:23 am, Lukas Bulwahn wrote:
>>>> 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: ?

So, I checked mailing list. Generated-by is used by the user to quote
script(s) and not the person.
E.g., Generated-by: scripts/coccinelle/api/alloc/kzalloc-simple.cocci

Maybe, it should be suggested to delete this tag.
What do you think?

>>>>
>>>> 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.
> 
These are some of the examples:
https://lore.kernel.org/lkml/20200904120257.464056467@linuxfoundation.org/

https://lore.kernel.org/lkml/20200904120257.464056467@linuxfoundation.org/

https://lore.kernel.org/lkml/20190507053235.29900-78-sashal@kernel.org/

Not sure if it is used as git blame or maybe to define the 'root' of
the tree at the time, etc


> 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?
> 
Yes, agree.

>> 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 at 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.
> 
Okay. One doubt though, do I need to create a separate patch for edit
distance fix? I am planning to create a hash for these typos(which do
not fulfill edit distance criteria) in checkpatch.pl itself.

What do you think?

> 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?
> 
Debugged-by: co-developed-by/reviewed-by/new tag

Originally-by: Co-developed-by / maybe a new tag

Bisected-by

Diagnosed-by

Root-caused-by: not sure if it is used as git blame or maybe to define
the 'root' of the tree at the time, etc

> Lukas
> 



More information about the Linux-kernel-mentees mailing list