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

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

[Aditya]

Hi Sir
I have analyzed the checkpatch report for BAD_SIGN_OFF(over
v4.13..v5.8) for non-standard signature and generated reports for it.
Some mistakes are more frequent than others, whereas some mistakes
even have a frequency of 1.

Non-standard signatures occurring with their frequency:
https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/non_standard_signs.txt

Complete warning messages:
https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/warn_msgs.txt

Should I implement the fix similar to TYPO_FIX, where we have a
separate file for common misspellings and corrected words? Or should I
make a hash of these misspellings in checkpatch.pl file as well?

Also should I include all these misspelled words in it? Or omit words
below certain frequency?

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

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

[Lukas Bulwahn]

On Wed, Nov 11, 2020 at 3:13 PM Aditya <yashsri421@gmail.com> wrote:
>
> Hi Sir
> I have analyzed the checkpatch report for BAD_SIGN_OFF(over
> v4.13..v5.8) for non-standard signature and generated reports for it.
> Some mistakes are more frequent than others, whereas some mistakes
> even have a frequency of 1.
>
> Non-standard signatures occurring with their frequency:
> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/non_standard_signs.txt
>
> Complete warning messages:
> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/warn_msgs.txt
>
> Should I implement the fix similar to TYPO_FIX, where we have a
> separate file for common misspellings and corrected words? Or should I
> make a hash of these misspellings in checkpatch.pl file as well?
>
> Also should I include all these misspelled words in it? Or omit words
> below certain frequency?
>

I think the best way would be to compute some kind of edit distance to
the known signature tags and if this edit distance is below a certain
threshold, suggest that signature tag as the fix. We can then evaluate
to determine the best suitable threshold. The edit distance between
the different tags are so large that this should always work as
intended.

Then, we can look into these other creative tags and propose suitable
existing tags for the more frequent ones that are non-standard. Or in
the case, none of the existing ones fit we can start the discussion on
proposing some new standard ones.

Lukas

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

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

[Aditya]

On 12/11/20 1:34 am, Lukas Bulwahn wrote:
> On Wed, Nov 11, 2020 at 3:13 PM Aditya <yashsri421@gmail.com> wrote:
>>
>> Hi Sir
>> I have analyzed the checkpatch report for BAD_SIGN_OFF(over
>> v4.13..v5.8) for non-standard signature and generated reports for it.
>> Some mistakes are more frequent than others, whereas some mistakes
>> even have a frequency of 1.
>>
>> Non-standard signatures occurring with their frequency:
>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/non_standard_signs.txt
>>
>> Complete warning messages:
>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/warn_msgs.txt
>>
>> Should I implement the fix similar to TYPO_FIX, where we have a
>> separate file for common misspellings and corrected words? Or should I
>> make a hash of these misspellings in checkpatch.pl file as well?
>>
>> Also should I include all these misspelled words in it? Or omit words
>> below certain frequency?
>>
> 
> I think the best way would be to compute some kind of edit distance to
> the known signature tags and if this edit distance is below a certain
> threshold, suggest that signature tag as the fix. We can then evaluate
> to determine the best suitable threshold. The edit distance between
> the different tags are so large that this should always work as
> intended.
> 
> Then, we can look into these other creative tags and propose suitable
> existing tags for the more frequent ones that are non-standard. Or in
> the case, none of the existing ones fit we can start the discussion on
> proposing some new standard ones.
> 

I have generated a list of non-standard signatures and their fixes on
the basis of edit distance.

This is the common list of non standard signatures and fixes (in
detail):
https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/min_dists.txt

As I observed, I think, we can consider '<=2' as the threshold edit
distance.
List for non-standard signature and their proposed fix with edit
distance<=2 :
https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/less_than_3.txt

I have also generated lists for 3 and 4 edit distance separately for
reference:
Equal to 3:
https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/equal_3.txt

Equal to 4:
https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/equal_4.txt

For the rest I guess we'll need to hard code eg. for 'Debugged-by',
'Requested-by' etc.

These are the complete lists of non-standard signatures:
https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/non_standard_signs.txt

What do you think?

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

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

[Aditya]

On 13/11/20 8:05 pm, Aditya wrote:
> On 12/11/20 1:34 am, Lukas Bulwahn wrote:
>> On Wed, Nov 11, 2020 at 3:13 PM Aditya <yashsri421@gmail.com> wrote:
>>>
>>> Hi Sir
>>> I have analyzed the checkpatch report for BAD_SIGN_OFF(over
>>> v4.13..v5.8) for non-standard signature and generated reports for it.
>>> Some mistakes are more frequent than others, whereas some mistakes
>>> even have a frequency of 1.
>>>
>>> Non-standard signatures occurring with their frequency:
>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/non_standard_signs.txt
>>>
>>> Complete warning messages:
>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/warn_msgs.txt
>>>
>>> Should I implement the fix similar to TYPO_FIX, where we have a
>>> separate file for common misspellings and corrected words? Or should I
>>> make a hash of these misspellings in checkpatch.pl file as well?
>>>
>>> Also should I include all these misspelled words in it? Or omit words
>>> below certain frequency?
>>>
>>
>> I think the best way would be to compute some kind of edit distance to
>> the known signature tags and if this edit distance is below a certain
>> threshold, suggest that signature tag as the fix. We can then evaluate
>> to determine the best suitable threshold. The edit distance between
>> the different tags are so large that this should always work as
>> intended.
>>
>> Then, we can look into these other creative tags and propose suitable
>> existing tags for the more frequent ones that are non-standard. Or in
>> the case, none of the existing ones fit we can start the discussion on
>> proposing some new standard ones.
>>
> 
> I have generated a list of non-standard signatures and their fixes on
> the basis of edit distance.
> 
> This is the common list of non standard signatures and fixes (in
> detail):
> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/min_dists.txt
> 
> As I observed, I think, we can consider '<=2' as the threshold edit
> distance.
> List for non-standard signature and their proposed fix with edit
> distance<=2 :
> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/less_than_3.txt
> 
> I have also generated lists for 3 and 4 edit distance separately for
> reference:
> Equal to 3:
> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/equal_3.txt
> 
> Equal to 4:
> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/equal_4.txt
> 
> For the rest I guess we'll need to hard code eg. for 'Debugged-by',
> 'Requested-by' etc.
> 
> These are the complete lists of non-standard signatures:
> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/non_standard_signs.txt
> 
> What do you think?
> 

For the calculation of edit distance, I have lower-cased the string
and removed hyphens from the string before comparison. This helps to
avoid increase in threshold for any misplaced hyphens

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

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

[Lukas Bulwahn]

On Fri, Nov 13, 2020 at 4:00 PM Aditya <yashsri421@gmail.com> wrote:
>
> On 13/11/20 8:05 pm, Aditya wrote:
> > On 12/11/20 1:34 am, Lukas Bulwahn wrote:
> >> On Wed, Nov 11, 2020 at 3:13 PM Aditya <yashsri421@gmail.com> wrote:
> >>>
> >>> Hi Sir
> >>> I have analyzed the checkpatch report for BAD_SIGN_OFF(over
> >>> v4.13..v5.8) for non-standard signature and generated reports for it.
> >>> Some mistakes are more frequent than others, whereas some mistakes
> >>> even have a frequency of 1.
> >>>
> >>> Non-standard signatures occurring with their frequency:
> >>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/non_standard_signs.txt
> >>>
> >>> Complete warning messages:
> >>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/warn_msgs.txt
> >>>
> >>> Should I implement the fix similar to TYPO_FIX, where we have a
> >>> separate file for common misspellings and corrected words? Or should I
> >>> make a hash of these misspellings in checkpatch.pl file as well?
> >>>
> >>> Also should I include all these misspelled words in it? Or omit words
> >>> below certain frequency?
> >>>
> >>
> >> I think the best way would be to compute some kind of edit distance to
> >> the known signature tags and if this edit distance is below a certain
> >> threshold, suggest that signature tag as the fix. We can then evaluate
> >> to determine the best suitable threshold. The edit distance between
> >> the different tags are so large that this should always work as
> >> intended.
> >>
> >> Then, we can look into these other creative tags and propose suitable
> >> existing tags for the more frequent ones that are non-standard. Or in
> >> the case, none of the existing ones fit we can start the discussion on
> >> proposing some new standard ones.
> >>
> >
> > I have generated a list of non-standard signatures and their fixes on
> > the basis of edit distance.
> >
> > This is the common list of non standard signatures and fixes (in
> > detail):
> > https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/min_dists.txt
> >
> > As I observed, I think, we can consider '<=2' as the threshold edit
> > distance.
> > List for non-standard signature and their proposed fix with edit
> > distance<=2 :
> > https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/less_than_3.txt
> >
> > I have also generated lists for 3 and 4 edit distance separately for
> > reference:
> > Equal to 3:
> > https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/equal_3.txt
> >
> > Equal to 4:
> > https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/equal_4.txt
> >
> > For the rest I guess we'll need to hard code eg. for 'Debugged-by',
> > 'Requested-by' etc.
> >
> > These are the complete lists of non-standard signatures:
> > https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/non_standard_signs.txt
> >

Can you share which non-standard-signatures would be
handled/transformed with edit distance 2 and which would not in a
similar format to non_standard_signs.txt (so, ordered by frequency).

We can then consider those that remain and find a good next strategy
for the most frequent non-standard signatures.

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

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

[Aditya]

On 13/11/20 8:56 pm, Lukas Bulwahn wrote:
> On Fri, Nov 13, 2020 at 4:00 PM Aditya <yashsri421@gmail.com> wrote:
>>
>> On 13/11/20 8:05 pm, Aditya wrote:
>>> On 12/11/20 1:34 am, Lukas Bulwahn wrote:
>>>> On Wed, Nov 11, 2020 at 3:13 PM Aditya <yashsri421@gmail.com> wrote:
>>>>>
>>>>> Hi Sir
>>>>> I have analyzed the checkpatch report for BAD_SIGN_OFF(over
>>>>> v4.13..v5.8) for non-standard signature and generated reports for it.
>>>>> Some mistakes are more frequent than others, whereas some mistakes
>>>>> even have a frequency of 1.
>>>>>
>>>>> Non-standard signatures occurring with their frequency:
>>>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/non_standard_signs.txt
>>>>>
>>>>> Complete warning messages:
>>>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/warn_msgs.txt
>>>>>
>>>>> Should I implement the fix similar to TYPO_FIX, where we have a
>>>>> separate file for common misspellings and corrected words? Or should I
>>>>> make a hash of these misspellings in checkpatch.pl file as well?
>>>>>
>>>>> Also should I include all these misspelled words in it? Or omit words
>>>>> below certain frequency?
>>>>>
>>>>
>>>> I think the best way would be to compute some kind of edit distance to
>>>> the known signature tags and if this edit distance is below a certain
>>>> threshold, suggest that signature tag as the fix. We can then evaluate
>>>> to determine the best suitable threshold. The edit distance between
>>>> the different tags are so large that this should always work as
>>>> intended.
>>>>
>>>> Then, we can look into these other creative tags and propose suitable
>>>> existing tags for the more frequent ones that are non-standard. Or in
>>>> the case, none of the existing ones fit we can start the discussion on
>>>> proposing some new standard ones.
>>>>
>>>
>>> I have generated a list of non-standard signatures and their fixes on
>>> the basis of edit distance.
>>>
>>> This is the common list of non standard signatures and fixes (in
>>> detail):
>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/min_dists.txt
>>>
>>> As I observed, I think, we can consider '<=2' as the threshold edit
>>> distance.
>>> List for non-standard signature and their proposed fix with edit
>>> distance<=2 :
>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/less_than_3.txt
>>>
>>> I have also generated lists for 3 and 4 edit distance separately for
>>> reference:
>>> Equal to 3:
>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/equal_3.txt
>>>
>>> Equal to 4:
>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/equal_4.txt
>>>
>>> For the rest I guess we'll need to hard code eg. for 'Debugged-by',
>>> 'Requested-by' etc.
>>>
>>> These are the complete lists of non-standard signatures:
>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/non_standard_signs.txt
>>>
> 
> Can you share which non-standard-signatures would be
> handled/transformed with edit distance 2 and which would not in a
> similar format to non_standard_signs.txt (so, ordered by frequency).
> 
> We can then consider those that remain and find a good next strategy
> for the most frequent non-standard signatures.
> 

Non standard signatures handled with edit distance 2:
https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/less_than2/signs_freq.txt

Non standard signatures with edit distance greater than 2:
https://github.com/AdityaSrivast/kernel-tasks/tree/master/random/non_standard_signature/more_than2

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

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

[Lukas Bulwahn]

On Tue, Nov 17, 2020 at 7:03 PM Aditya <yashsri421@gmail.com> wrote:
>
> On 13/11/20 11:55 pm, Aditya wrote:
> > On 13/11/20 8:56 pm, Lukas Bulwahn wrote:
> >> On Fri, Nov 13, 2020 at 4:00 PM Aditya <yashsri421@gmail.com> wrote:
> >>>
> >>> On 13/11/20 8:05 pm, Aditya wrote:
> >>>> On 12/11/20 1:34 am, Lukas Bulwahn wrote:
> >>>>> On Wed, Nov 11, 2020 at 3:13 PM Aditya <yashsri421@gmail.com> wrote:
> >>>>>>
> >>>>>> Hi Sir
> >>>>>> I have analyzed the checkpatch report for BAD_SIGN_OFF(over
> >>>>>> v4.13..v5.8) for non-standard signature and generated reports for it.
> >>>>>> Some mistakes are more frequent than others, whereas some mistakes
> >>>>>> even have a frequency of 1.
> >>>>>>
> >>>>>> Non-standard signatures occurring with their frequency:
> >>>>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/non_standard_signs.txt
> >>>>>>
> >>>>>> Complete warning messages:
> >>>>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/warn_msgs.txt
> >>>>>>
> >>>>>> Should I implement the fix similar to TYPO_FIX, where we have a
> >>>>>> separate file for common misspellings and corrected words? Or should I
> >>>>>> make a hash of these misspellings in checkpatch.pl file as well?
> >>>>>>
> >>>>>> Also should I include all these misspelled words in it? Or omit words
> >>>>>> below certain frequency?
> >>>>>>
> >>>>>
> >>>>> I think the best way would be to compute some kind of edit distance to
> >>>>> the known signature tags and if this edit distance is below a certain
> >>>>> threshold, suggest that signature tag as the fix. We can then evaluate
> >>>>> to determine the best suitable threshold. The edit distance between
> >>>>> the different tags are so large that this should always work as
> >>>>> intended.
> >>>>>
> >>>>> Then, we can look into these other creative tags and propose suitable
> >>>>> existing tags for the more frequent ones that are non-standard. Or in
> >>>>> the case, none of the existing ones fit we can start the discussion on
> >>>>> proposing some new standard ones.
> >>>>>
> >>>>
> >>>> I have generated a list of non-standard signatures and their fixes on
> >>>> the basis of edit distance.
> >>>>
> >>>> This is the common list of non standard signatures and fixes (in
> >>>> detail):
> >>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/min_dists.txt
> >>>>
> >>>> As I observed, I think, we can consider '<=2' as the threshold edit
> >>>> distance.
> >>>> List for non-standard signature and their proposed fix with edit
> >>>> distance<=2 :
> >>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/less_than_3.txt
> >>>>
> >>>> I have also generated lists for 3 and 4 edit distance separately for
> >>>> reference:
> >>>> Equal to 3:
> >>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/equal_3.txt
> >>>>
> >>>> Equal to 4:
> >>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/equal_4.txt
> >>>>
> >>>> For the rest I guess we'll need to hard code eg. for 'Debugged-by',
> >>>> 'Requested-by' etc.
> >>>>
> >>>> These are the complete lists of non-standard signatures:
> >>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/non_standard_signs.txt
> >>>>
> >>
> >> Can you share which non-standard-signatures would be
> >> handled/transformed with edit distance 2 and which would not in a
> >> similar format to non_standard_signs.txt (so, ordered by frequency).
> >>
> >> We can then consider those that remain and find a good next strategy
> >> for the most frequent non-standard signatures.
> >>
> >
> > Non standard signatures handled with edit distance 2:
> > https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/less_than2/signs_freq.txt
> >
> > Non standard signatures with edit distance greater than 2:
> > https://github.com/AdityaSrivast/kernel-tasks/tree/master/random/non_standard_signature/more_than2
> >
>
> I think this mail probably got missed. I'll summarize it a bit for
> simplicity:
> With edit distance approach and threshold as 2, we're able to handle
> 39 out of 109 'distinct' cases of non-standard signature. In this 39,
> the maximum count of non-standard signature is 19 for 'Reviwed-by:'; 9
> for 'Reviewd-by:' and other common mispellings.
> Complete List:
> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/less_than2/signs_freq.txt
>
> However, still we are unable to account for 70 non-standard signatures
> which occur more frequently (eg 'Debugged-by:', which has occurred 61
> times; 'Requested-by:', 48 times; and so on).
> Complete list:
> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/more_than2/signs_freq.txt
>
> I think for these cases we'd need to make some file (as is used for
> TYPO_SPELLING), or hash.
> What do you think/suggest?
>

Yes, I agree.

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?

Lukas

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

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

[Aditya]

On 13/11/20 11:55 pm, Aditya wrote:
> On 13/11/20 8:56 pm, Lukas Bulwahn wrote:
>> On Fri, Nov 13, 2020 at 4:00 PM Aditya <yashsri421@gmail.com> wrote:
>>>
>>> On 13/11/20 8:05 pm, Aditya wrote:
>>>> On 12/11/20 1:34 am, Lukas Bulwahn wrote:
>>>>> On Wed, Nov 11, 2020 at 3:13 PM Aditya <yashsri421@gmail.com> wrote:
>>>>>>
>>>>>> Hi Sir
>>>>>> I have analyzed the checkpatch report for BAD_SIGN_OFF(over
>>>>>> v4.13..v5.8) for non-standard signature and generated reports for it.
>>>>>> Some mistakes are more frequent than others, whereas some mistakes
>>>>>> even have a frequency of 1.
>>>>>>
>>>>>> Non-standard signatures occurring with their frequency:
>>>>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/non_standard_signs.txt
>>>>>>
>>>>>> Complete warning messages:
>>>>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/warn_msgs.txt
>>>>>>
>>>>>> Should I implement the fix similar to TYPO_FIX, where we have a
>>>>>> separate file for common misspellings and corrected words? Or should I
>>>>>> make a hash of these misspellings in checkpatch.pl file as well?
>>>>>>
>>>>>> Also should I include all these misspelled words in it? Or omit words
>>>>>> below certain frequency?
>>>>>>
>>>>>
>>>>> I think the best way would be to compute some kind of edit distance to
>>>>> the known signature tags and if this edit distance is below a certain
>>>>> threshold, suggest that signature tag as the fix. We can then evaluate
>>>>> to determine the best suitable threshold. The edit distance between
>>>>> the different tags are so large that this should always work as
>>>>> intended.
>>>>>
>>>>> Then, we can look into these other creative tags and propose suitable
>>>>> existing tags for the more frequent ones that are non-standard. Or in
>>>>> the case, none of the existing ones fit we can start the discussion on
>>>>> proposing some new standard ones.
>>>>>
>>>>
>>>> I have generated a list of non-standard signatures and their fixes on
>>>> the basis of edit distance.
>>>>
>>>> This is the common list of non standard signatures and fixes (in
>>>> detail):
>>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/min_dists.txt
>>>>
>>>> As I observed, I think, we can consider '<=2' as the threshold edit
>>>> distance.
>>>> List for non-standard signature and their proposed fix with edit
>>>> distance<=2 :
>>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/less_than_3.txt
>>>>
>>>> I have also generated lists for 3 and 4 edit distance separately for
>>>> reference:
>>>> Equal to 3:
>>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/equal_3.txt
>>>>
>>>> Equal to 4:
>>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/equal_4.txt
>>>>
>>>> For the rest I guess we'll need to hard code eg. for 'Debugged-by',
>>>> 'Requested-by' etc.
>>>>
>>>> These are the complete lists of non-standard signatures:
>>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/non_standard_signs.txt
>>>>
>>
>> Can you share which non-standard-signatures would be
>> handled/transformed with edit distance 2 and which would not in a
>> similar format to non_standard_signs.txt (so, ordered by frequency).
>>
>> We can then consider those that remain and find a good next strategy
>> for the most frequent non-standard signatures.
>>
> 
> Non standard signatures handled with edit distance 2:
> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/less_than2/signs_freq.txt
> 
> Non standard signatures with edit distance greater than 2:
> https://github.com/AdityaSrivast/kernel-tasks/tree/master/random/non_standard_signature/more_than2
> 

I think this mail probably got missed. I'll summarize it a bit for
simplicity:
With edit distance approach and threshold as 2, we're able to handle
39 out of 109 'distinct' cases of non-standard signature. In this 39,
the maximum count of non-standard signature is 19 for 'Reviwed-by:'; 9
for 'Reviewd-by:' and other common mispellings.
Complete List:
https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/less_than2/signs_freq.txt

However, still we are unable to account for 70 non-standard signatures
which occur more frequently (eg 'Debugged-by:', which has occurred 61
times; 'Requested-by:', 48 times; and so on).
Complete list:
https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/more_than2/signs_freq.txt

I think for these cases we'd need to make some file (as is used for
TYPO_SPELLING), or hash.
What do you think/suggest?

Thanks
Aditya

> Thanks
> Aditya
> 

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

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

[Aditya]

On 17/11/20 11:12 pm, Lukas Bulwahn wrote:
> On Tue, Nov 17, 2020 at 7:03 PM Aditya <yashsri421@gmail.com> wrote:
>>
>> On 13/11/20 11:55 pm, Aditya wrote:
>>> On 13/11/20 8:56 pm, Lukas Bulwahn wrote:
>>>> On Fri, Nov 13, 2020 at 4:00 PM Aditya <yashsri421@gmail.com> wrote:
>>>>>
>>>>> On 13/11/20 8:05 pm, Aditya wrote:
>>>>>> On 12/11/20 1:34 am, Lukas Bulwahn wrote:
>>>>>>> On Wed, Nov 11, 2020 at 3:13 PM Aditya <yashsri421@gmail.com> wrote:
>>>>>>>>
>>>>>>>> Hi Sir
>>>>>>>> I have analyzed the checkpatch report for BAD_SIGN_OFF(over
>>>>>>>> v4.13..v5.8) for non-standard signature and generated reports for it.
>>>>>>>> Some mistakes are more frequent than others, whereas some mistakes
>>>>>>>> even have a frequency of 1.
>>>>>>>>
>>>>>>>> Non-standard signatures occurring with their frequency:
>>>>>>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/non_standard_signs.txt
>>>>>>>>
>>>>>>>> Complete warning messages:
>>>>>>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/warn_msgs.txt
>>>>>>>>
>>>>>>>> Should I implement the fix similar to TYPO_FIX, where we have a
>>>>>>>> separate file for common misspellings and corrected words? Or should I
>>>>>>>> make a hash of these misspellings in checkpatch.pl file as well?
>>>>>>>>
>>>>>>>> Also should I include all these misspelled words in it? Or omit words
>>>>>>>> below certain frequency?
>>>>>>>>
>>>>>>>
>>>>>>> I think the best way would be to compute some kind of edit distance to
>>>>>>> the known signature tags and if this edit distance is below a certain
>>>>>>> threshold, suggest that signature tag as the fix. We can then evaluate
>>>>>>> to determine the best suitable threshold. The edit distance between
>>>>>>> the different tags are so large that this should always work as
>>>>>>> intended.
>>>>>>>
>>>>>>> Then, we can look into these other creative tags and propose suitable
>>>>>>> existing tags for the more frequent ones that are non-standard. Or in
>>>>>>> the case, none of the existing ones fit we can start the discussion on
>>>>>>> proposing some new standard ones.
>>>>>>>
>>>>>>
>>>>>> I have generated a list of non-standard signatures and their fixes on
>>>>>> the basis of edit distance.
>>>>>>
>>>>>> This is the common list of non standard signatures and fixes (in
>>>>>> detail):
>>>>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/min_dists.txt
>>>>>>
>>>>>> As I observed, I think, we can consider '<=2' as the threshold edit
>>>>>> distance.
>>>>>> List for non-standard signature and their proposed fix with edit
>>>>>> distance<=2 :
>>>>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/less_than_3.txt
>>>>>>
>>>>>> I have also generated lists for 3 and 4 edit distance separately for
>>>>>> reference:
>>>>>> Equal to 3:
>>>>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/equal_3.txt
>>>>>>
>>>>>> Equal to 4:
>>>>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/equal_4.txt
>>>>>>
>>>>>> For the rest I guess we'll need to hard code eg. for 'Debugged-by',
>>>>>> 'Requested-by' etc.
>>>>>>
>>>>>> These are the complete lists of non-standard signatures:
>>>>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/non_standard_signs.txt
>>>>>>
>>>>
>>>> Can you share which non-standard-signatures would be
>>>> handled/transformed with edit distance 2 and which would not in a
>>>> similar format to non_standard_signs.txt (so, ordered by frequency).
>>>>
>>>> We can then consider those that remain and find a good next strategy
>>>> for the most frequent non-standard signatures.
>>>>
>>>
>>> Non standard signatures handled with edit distance 2:
>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/less_than2/signs_freq.txt
>>>
>>> Non standard signatures with edit distance greater than 2:
>>> https://github.com/AdityaSrivast/kernel-tasks/tree/master/random/non_standard_signature/more_than2
>>>
>>
>> I think this mail probably got missed. I'll summarize it a bit for
>> simplicity:
>> With edit distance approach and threshold as 2, we're able to handle
>> 39 out of 109 'distinct' cases of non-standard signature. In this 39,
>> the maximum count of non-standard signature is 19 for 'Reviwed-by:'; 9
>> for 'Reviewd-by:' and other common mispellings.
>> Complete List:
>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/less_than2/signs_freq.txt
>>
>> However, still we are unable to account for 70 non-standard signatures
>> which occur more frequently (eg 'Debugged-by:', which has occurred 61
>> times; 'Requested-by:', 48 times; and so on).
>> Complete list:
>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/more_than2/signs_freq.txt
>>
>> I think for these cases we'd need to make some file (as is used for
>> TYPO_SPELLING), or hash.
>> What do you think/suggest?
>>
> 
> Yes, I agree.
> 
> 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?
> 

Sounds good. Will send by tomorrow morning :)

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

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

[Aditya]

On 18/11/20 2:24 am, Aditya wrote:
> On 17/11/20 11:12 pm, Lukas Bulwahn wrote:
>> On Tue, Nov 17, 2020 at 7:03 PM Aditya <yashsri421@gmail.com> wrote:
>>>
>>> On 13/11/20 11:55 pm, Aditya wrote:
>>>> On 13/11/20 8:56 pm, Lukas Bulwahn wrote:
>>>>> On Fri, Nov 13, 2020 at 4:00 PM Aditya <yashsri421@gmail.com> wrote:
>>>>>>
>>>>>> On 13/11/20 8:05 pm, Aditya wrote:
>>>>>>> On 12/11/20 1:34 am, Lukas Bulwahn wrote:
>>>>>>>> On Wed, Nov 11, 2020 at 3:13 PM Aditya <yashsri421@gmail.com> wrote:
>>>>>>>>>
>>>>>>>>> Hi Sir
>>>>>>>>> I have analyzed the checkpatch report for BAD_SIGN_OFF(over
>>>>>>>>> v4.13..v5.8) for non-standard signature and generated reports for it.
>>>>>>>>> Some mistakes are more frequent than others, whereas some mistakes
>>>>>>>>> even have a frequency of 1.
>>>>>>>>>
>>>>>>>>> Non-standard signatures occurring with their frequency:
>>>>>>>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/non_standard_signs.txt
>>>>>>>>>
>>>>>>>>> Complete warning messages:
>>>>>>>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/warn_msgs.txt
>>>>>>>>>
>>>>>>>>> Should I implement the fix similar to TYPO_FIX, where we have a
>>>>>>>>> separate file for common misspellings and corrected words? Or should I
>>>>>>>>> make a hash of these misspellings in checkpatch.pl file as well?
>>>>>>>>>
>>>>>>>>> Also should I include all these misspelled words in it? Or omit words
>>>>>>>>> below certain frequency?
>>>>>>>>>
>>>>>>>>
>>>>>>>> I think the best way would be to compute some kind of edit distance to
>>>>>>>> the known signature tags and if this edit distance is below a certain
>>>>>>>> threshold, suggest that signature tag as the fix. We can then evaluate
>>>>>>>> to determine the best suitable threshold. The edit distance between
>>>>>>>> the different tags are so large that this should always work as
>>>>>>>> intended.
>>>>>>>>
>>>>>>>> Then, we can look into these other creative tags and propose suitable
>>>>>>>> existing tags for the more frequent ones that are non-standard. Or in
>>>>>>>> the case, none of the existing ones fit we can start the discussion on
>>>>>>>> proposing some new standard ones.
>>>>>>>>
>>>>>>>
>>>>>>> I have generated a list of non-standard signatures and their fixes on
>>>>>>> the basis of edit distance.
>>>>>>>
>>>>>>> This is the common list of non standard signatures and fixes (in
>>>>>>> detail):
>>>>>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/min_dists.txt
>>>>>>>
>>>>>>> As I observed, I think, we can consider '<=2' as the threshold edit
>>>>>>> distance.
>>>>>>> List for non-standard signature and their proposed fix with edit
>>>>>>> distance<=2 :
>>>>>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/less_than_3.txt
>>>>>>>
>>>>>>> I have also generated lists for 3 and 4 edit distance separately for
>>>>>>> reference:
>>>>>>> Equal to 3:
>>>>>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/equal_3.txt
>>>>>>>
>>>>>>> Equal to 4:
>>>>>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/equal_4.txt
>>>>>>>
>>>>>>> For the rest I guess we'll need to hard code eg. for 'Debugged-by',
>>>>>>> 'Requested-by' etc.
>>>>>>>
>>>>>>> These are the complete lists of non-standard signatures:
>>>>>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/non_standard_signs.txt
>>>>>>>
>>>>>
>>>>> Can you share which non-standard-signatures would be
>>>>> handled/transformed with edit distance 2 and which would not in a
>>>>> similar format to non_standard_signs.txt (so, ordered by frequency).
>>>>>
>>>>> We can then consider those that remain and find a good next strategy
>>>>> for the most frequent non-standard signatures.
>>>>>
>>>>
>>>> Non standard signatures handled with edit distance 2:
>>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/less_than2/signs_freq.txt
>>>>
>>>> Non standard signatures with edit distance greater than 2:
>>>> https://github.com/AdityaSrivast/kernel-tasks/tree/master/random/non_standard_signature/more_than2
>>>>
>>>
>>> I think this mail probably got missed. I'll summarize it a bit for
>>> simplicity:
>>> With edit distance approach and threshold as 2, we're able to handle
>>> 39 out of 109 'distinct' cases of non-standard signature. In this 39,
>>> the maximum count of non-standard signature is 19 for 'Reviwed-by:'; 9
>>> for 'Reviewd-by:' and other common mispellings.
>>> Complete List:
>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/less_than2/signs_freq.txt
>>>
>>> However, still we are unable to account for 70 non-standard signatures
>>> which occur more frequently (eg 'Debugged-by:', which has occurred 61
>>> times; 'Requested-by:', 48 times; and so on).
>>> Complete list:
>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/more_than2/signs_freq.txt
>>>
>>> I think for these cases we'd need to make some file (as is used for
>>> TYPO_SPELLING), or hash.
>>> What do you think/suggest?
>>>
>>
>> Yes, I agree.
>>
>> 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

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.

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/

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/

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

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

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

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

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

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

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

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

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/

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

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.

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

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

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/

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.

What do you think?

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

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

[Lukas Bulwahn]

[-- Attachment #1.1: Type: text/plain, Size: 11707 bytes --]

On Mi., 18. Nov. 2020 at 11:12, Aditya <yashsri421@gmail.com> wrote:

> On 18/11/20 2:24 am, Aditya wrote:
> > On 17/11/20 11:12 pm, Lukas Bulwahn wrote:
> >> On Tue, Nov 17, 2020 at 7:03 PM Aditya <yashsri421@gmail.com> wrote:
> >>>
> >>> On 13/11/20 11:55 pm, Aditya wrote:
> >>>> On 13/11/20 8:56 pm, Lukas Bulwahn wrote:
> >>>>> On Fri, Nov 13, 2020 at 4:00 PM Aditya <yashsri421@gmail.com> wrote:
> >>>>>>
> >>>>>> On 13/11/20 8:05 pm, Aditya wrote:
> >>>>>>> On 12/11/20 1:34 am, Lukas Bulwahn wrote:
> >>>>>>>> On Wed, Nov 11, 2020 at 3:13 PM Aditya <yashsri421@gmail.com>
> wrote:
> >>>>>>>>>
> >>>>>>>>> Hi Sir
> >>>>>>>>> I have analyzed the checkpatch report for BAD_SIGN_OFF(over
> >>>>>>>>> v4.13..v5.8) for non-standard signature and generated reports
> for it.
> >>>>>>>>> Some mistakes are more frequent than others, whereas some
> mistakes
> >>>>>>>>> even have a frequency of 1.
> >>>>>>>>>
> >>>>>>>>> Non-standard signatures occurring with their frequency:
> >>>>>>>>>
> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/non_standard_signs.txt
> >>>>>>>>>
> >>>>>>>>> Complete warning messages:
> >>>>>>>>>
> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/warn_msgs.txt
> >>>>>>>>>
> >>>>>>>>> Should I implement the fix similar to TYPO_FIX, where we have a
> >>>>>>>>> separate file for common misspellings and corrected words? Or
> should I
> >>>>>>>>> make a hash of these misspellings in checkpatch.pl file as well?
> >>>>>>>>>
> >>>>>>>>> Also should I include all these misspelled words in it? Or omit
> words
> >>>>>>>>> below certain frequency?
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>> I think the best way would be to compute some kind of edit
> distance to
> >>>>>>>> the known signature tags and if this edit distance is below a
> certain
> >>>>>>>> threshold, suggest that signature tag as the fix. We can then
> evaluate
> >>>>>>>> to determine the best suitable threshold. The edit distance
> between
> >>>>>>>> the different tags are so large that this should always work as
> >>>>>>>> intended.
> >>>>>>>>
> >>>>>>>> Then, we can look into these other creative tags and propose
> suitable
> >>>>>>>> existing tags for the more frequent ones that are non-standard.
> Or in
> >>>>>>>> the case, none of the existing ones fit we can start the
> discussion on
> >>>>>>>> proposing some new standard ones.
> >>>>>>>>
> >>>>>>>
> >>>>>>> I have generated a list of non-standard signatures and their fixes
> on
> >>>>>>> the basis of edit distance.
> >>>>>>>
> >>>>>>> This is the common list of non standard signatures and fixes (in
> >>>>>>> detail):
> >>>>>>>
> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/min_dists.txt
> >>>>>>>
> >>>>>>> As I observed, I think, we can consider '<=2' as the threshold edit
> >>>>>>> distance.
> >>>>>>> List for non-standard signature and their proposed fix with edit
> >>>>>>> distance<=2 :
> >>>>>>>
> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/less_than_3.txt
> >>>>>>>
> >>>>>>> I have also generated lists for 3 and 4 edit distance separately
> for
> >>>>>>> reference:
> >>>>>>> Equal to 3:
> >>>>>>>
> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/equal_3.txt
> >>>>>>>
> >>>>>>> Equal to 4:
> >>>>>>>
> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/equal_4.txt
> >>>>>>>
> >>>>>>> For the rest I guess we'll need to hard code eg. for 'Debugged-by',
> >>>>>>> 'Requested-by' etc.
> >>>>>>>
> >>>>>>> These are the complete lists of non-standard signatures:
> >>>>>>>
> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/non_standard_signs.txt
> >>>>>>>
> >>>>>
> >>>>> Can you share which non-standard-signatures would be
> >>>>> handled/transformed with edit distance 2 and which would not in a
> >>>>> similar format to non_standard_signs.txt (so, ordered by frequency).
> >>>>>
> >>>>> We can then consider those that remain and find a good next strategy
> >>>>> for the most frequent non-standard signatures.
> >>>>>
> >>>>
> >>>> Non standard signatures handled with edit distance 2:
> >>>>
> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/less_than2/signs_freq.txt
> >>>>
> >>>> Non standard signatures with edit distance greater than 2:
> >>>>
> https://github.com/AdityaSrivast/kernel-tasks/tree/master/random/non_standard_signature/more_than2
> >>>>
> >>>
> >>> I think this mail probably got missed. I'll summarize it a bit for
> >>> simplicity:
> >>> With edit distance approach and threshold as 2, we're able to handle
> >>> 39 out of 109 'distinct' cases of non-standard signature. In this 39,
> >>> the maximum count of non-standard signature is 19 for 'Reviwed-by:'; 9
> >>> for 'Reviewd-by:' and other common mispellings.
> >>> Complete List:
> >>>
> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/less_than2/signs_freq.txt
> >>>
> >>> However, still we are unable to account for 70 non-standard signatures
> >>> which occur more frequently (eg 'Debugged-by:', which has occurred 61
> >>> times; 'Requested-by:', 48 times; and so on).
> >>> Complete list:
> >>>
> https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/non_standard_signature/more_than2/signs_freq.txt
> >>>
> >>> I think for these cases we'd need to make some file (as is used for
> >>> TYPO_SPELLING), or hash.
> >>> What do you think/suggest?
> >>>
> >>
> >> Yes, I agree.
> >>
> >> 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/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
>
> 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.
>
> 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/
>
> 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/
>
> 16)Based-on-patch-by: 7 -> Similar to (13) Based-on-a-patch-by
>
> 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
>
> 18)Original-by: 6 -> Similar to '(4)Originally-by'
>
> 19)Acked-for-MFD-by: 6 -> Acked-by:
>
> 20)Reviewed-off-by: 5 -> Reviewed-by:
>
> 21)Based-on-patches-by: 5 -> Similar to (13)
>
> 22)Analysed-by: 5 -> Co-developed-by/Reviewed-by
> Rationale: Similar to '(5)Analyzed-by'
>
> 23)Based-on-work-by: 5 -> Not sure. Maybe 'Suggested-by'
>
> 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/
>
> 25)Reported-and-bisected-by: 4 -> Two different tags: 'Reported-by:'
> and 'Bisected-by'
>
> 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.
>
> 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
>
> 28)Suggestions-by: 3 -> Suggested-by
>
> 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/
>
> 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.
>
> What do you think?
>

I will provide some detailed comments in a few days, okay?

I suggest you start creating the patch that fixes tags with the low edit
distance.

Once that patch is good and accepted, we continue with the work on this
list.

Lukas


> Thanks
> Aditya
>

[-- Attachment #1.2: Type: text/html, Size: 18695 bytes --]

[-- Attachment #2: Type: text/plain, Size: 201 bytes --]

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

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

[Lukas Bulwahn]

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

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

[Aditya]

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@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
> 

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

[Linux-kernel-mentees] [PATCH] checkpatch: add fix and improve warning msg for Non-standard signature

[Aditya Srivastava]

Checkpatch.pl warns on non-standard signature styles.

E.g., running checkpatch on commit 513f7f747e1c ("parisc: Fix vmap
memory leak in ioremap()/iounmap()") reports this warning:

WARNING: Non-standard signature: Noticed-by:
Noticed-by: Sven Schnelle <svens@stackframe.org>

Provide a fix by:
1) replacing the non-standard signature with its standard equivalent
2) removing the signature if it is not required

Also, improve warning messages correspondingly, providing users
suggestions to either replace or remove the signature

Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
---
 scripts/checkpatch.pl | 45 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 43 insertions(+), 2 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index fdfd5ec09be6..23a21dc2c29a 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -506,6 +506,27 @@ our $signature_tags = qr{(?xi:
 	Cc:
 )};
 
+our %standard_signature_fix = (
+	"Requested-by:" => "Suggested-by:",
+	"Co-authored-by:" => "Co-developed-by:",
+	"Analyzed-by:" => "Co-developed-by:",
+	"Analysed-by:" => "Co-developed-by:",
+	"Improvements-by:" => "Co-developed-by:",
+	"Noticed-by:" => "Reported-by:",
+	"Inspired-by:" => "Suggested-by:",
+	"Verified-by:" => "Tested-by:",
+	"Okay-ished-by:" => "Acked-by:",
+	"Acked-for-MFD-by:" => "Acked-by:",
+	"Reviewed-off-by:" => "Reviewed-by:",
+	"Proposed-by:" => "Suggested-by:",
+	"Fixed-by:" => "Co-developed-by:",
+	"Pointed-out-by:" => "Suggested-by:",
+	"Pointed-at-by:" => "Suggested-by:",
+	"Suggestions-by:" => "Suggested-by:",
+	"Generated-by:" => "remove",
+	"Celebrated-by:" => "remove",
+);
+
 our @typeListMisordered = (
 	qr{char\s+(?:un)?signed},
 	qr{int\s+(?:(?:un)?signed\s+)?short\s},
@@ -2773,8 +2794,28 @@ sub process {
 			my $ucfirst_sign_off = ucfirst(lc($sign_off));
 
 			if ($sign_off !~ /$signature_tags/) {
-				WARN("BAD_SIGN_OFF",
-				     "Non-standard signature: $sign_off\n" . $herecurr);
+				my $suggested_signature = "";
+				if (exists($standard_signature_fix{$sign_off})) {
+					$suggested_signature = $standard_signature_fix{$sign_off};
+				}
+				if ($suggested_signature eq "") {
+					WARN("BAD_SIGN_OFF",
+					     "Non-standard signature: $sign_off\n" . $herecurr);
+				}
+				elsif ($suggested_signature eq "remove") {
+					if (WARN("BAD_SIGN_OFF",
+						"Non-standard signature: $sign_off. Please consider removing this signature tag.\n" . $herecurr) &&
+					$fix) {
+						fix_delete_line($fixlinenr, $rawline);
+					}
+				}
+				else {
+					if (WARN("BAD_SIGN_OFF",
+						"Non-standard signature: $sign_off. Please use '$suggested_signature' instead.\n" . $herecurr) &&
+					$fix) {
+						$fixed[$fixlinenr] =~ s/$sign_off/$suggested_signature/;
+					}
+				}
 			}
 			if (defined $space_before && $space_before ne "") {
 				if (WARN("BAD_SIGN_OFF",
-- 
2.17.1

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

Re: [Linux-kernel-mentees] [PATCH] checkpatch: add fix and improve warning msg for Non-standard signature

[Aditya]

On 21/11/20 1:28 am, Aditya Srivastava wrote:
> Checkpatch.pl warns on non-standard signature styles.
> 
> E.g., running checkpatch on commit 513f7f747e1c ("parisc: Fix vmap
> memory leak in ioremap()/iounmap()") reports this warning:
> 
> WARNING: Non-standard signature: Noticed-by:
> Noticed-by: Sven Schnelle <svens@stackframe.org>
> 
> Provide a fix by:
> 1) replacing the non-standard signature with its standard equivalent
> 2) removing the signature if it is not required
> 
> Also, improve warning messages correspondingly, providing users
> suggestions to either replace or remove the signature
> 
> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
> ---
>  scripts/checkpatch.pl | 45 +++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 43 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index fdfd5ec09be6..23a21dc2c29a 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -506,6 +506,27 @@ our $signature_tags = qr{(?xi:
>  	Cc:
>  )};
>  
> +our %standard_signature_fix = (
> +	"Requested-by:" => "Suggested-by:",
> +	"Co-authored-by:" => "Co-developed-by:",
> +	"Analyzed-by:" => "Co-developed-by:",
> +	"Analysed-by:" => "Co-developed-by:",
> +	"Improvements-by:" => "Co-developed-by:",
> +	"Noticed-by:" => "Reported-by:",
> +	"Inspired-by:" => "Suggested-by:",
> +	"Verified-by:" => "Tested-by:",
> +	"Okay-ished-by:" => "Acked-by:",
> +	"Acked-for-MFD-by:" => "Acked-by:",
> +	"Reviewed-off-by:" => "Reviewed-by:",
> +	"Proposed-by:" => "Suggested-by:",
> +	"Fixed-by:" => "Co-developed-by:",
> +	"Pointed-out-by:" => "Suggested-by:",
> +	"Pointed-at-by:" => "Suggested-by:",
> +	"Suggestions-by:" => "Suggested-by:",
> +	"Generated-by:" => "remove",
> +	"Celebrated-by:" => "remove",
> +);
> +
>  our @typeListMisordered = (
>  	qr{char\s+(?:un)?signed},
>  	qr{int\s+(?:(?:un)?signed\s+)?short\s},
> @@ -2773,8 +2794,28 @@ sub process {
>  			my $ucfirst_sign_off = ucfirst(lc($sign_off));
>  
>  			if ($sign_off !~ /$signature_tags/) {
> -				WARN("BAD_SIGN_OFF",
> -				     "Non-standard signature: $sign_off\n" . $herecurr);
> +				my $suggested_signature = "";
> +				if (exists($standard_signature_fix{$sign_off})) {
> +					$suggested_signature = $standard_signature_fix{$sign_off};
> +				}
> +				if ($suggested_signature eq "") {
> +					WARN("BAD_SIGN_OFF",
> +					     "Non-standard signature: $sign_off\n" . $herecurr);
> +				}
> +				elsif ($suggested_signature eq "remove") {
> +					if (WARN("BAD_SIGN_OFF",
> +						"Non-standard signature: $sign_off. Please consider removing this signature tag.\n" . $herecurr) &&
> +					$fix) {
> +						fix_delete_line($fixlinenr, $rawline);
> +					}
> +				}
> +				else {
> +					if (WARN("BAD_SIGN_OFF",
> +						"Non-standard signature: $sign_off. Please use '$suggested_signature' instead.\n" . $herecurr) &&
> +					$fix) {
> +						$fixed[$fixlinenr] =~ s/$sign_off/$suggested_signature/;
> +					}
> +				}
>  			}
>  			if (defined $space_before && $space_before ne "") {
>  				if (WARN("BAD_SIGN_OFF",
> 

Initial tests performed on patches found this fix to be working as
expected.

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

Re: [Linux-kernel-mentees] [PATCH] checkpatch: add fix and improve warning msg for Non-standard signature

[Lukas Bulwahn]

[-- Attachment #1.1: Type: text/plain, Size: 4469 bytes --]

On Fr., 20. Nov. 2020 at 21:03, Aditya <yashsri421@gmail.com> wrote:

> On 21/11/20 1:28 am, Aditya Srivastava wrote:
> > Checkpatch.pl warns on non-standard signature styles.
> >
> > E.g., running checkpatch on commit 513f7f747e1c ("parisc: Fix vmap
> > memory leak in ioremap()/iounmap()") reports this warning:
> >
> > WARNING: Non-standard signature: Noticed-by:
> > Noticed-by: Sven Schnelle <svens@stackframe.org>
> >


This example really does not tell anyone much.

Replace it with a summary from your evaluation.


> > Provide a fix by:
> > 1) replacing the non-standard signature with its standard equivalent
> > 2) removing the signature if it is not required
> >
> > Also, improve warning messages correspondingly, providing users
> > suggestions to either replace or remove the signature
> >


Looks good.



> > Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
> > ---
> >  scripts/checkpatch.pl | 45 +++++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 43 insertions(+), 2 deletions(-)
> >
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > index fdfd5ec09be6..23a21dc2c29a 100755
> > --- a/scripts/checkpatch.pl
> > +++ b/scripts/checkpatch.pl
> > @@ -506,6 +506,27 @@ our $signature_tags = qr{(?xi:
> >       Cc:
> >  )};
> >
> > +our %standard_signature_fix = (
> > +     "Requested-by:" => "Suggested-by:",
> > +     "Co-authored-by:" => "Co-developed-by:",
> > +     "Analyzed-by:" => "Co-developed-by:",
> > +     "Analysed-by:" => "Co-developed-by:",
> > +     "Improvements-by:" => "Co-developed-by:",
> > +     "Noticed-by:" => "Reported-by:",
> > +     "Inspired-by:" => "Suggested-by:",
> > +     "Verified-by:" => "Tested-by:",
> > +     "Okay-ished-by:" => "Acked-by:",
> > +     "Acked-for-MFD-by:" => "Acked-by:",
> > +     "Reviewed-off-by:" => "Reviewed-by:",
> > +     "Proposed-by:" => "Suggested-by:",
> > +     "Fixed-by:" => "Co-developed-by:",
> > +     "Pointed-out-by:" => "Suggested-by:",
> > +     "Pointed-at-by:" => "Suggested-by:",
> > +     "Suggestions-by:" => "Suggested-by:",
> > +     "Generated-by:" => "remove",
> > +     "Celebrated-by:" => "remove",
> > +);
> > +


How did create this list? I thought we looked at 30 cases...


> >  our @typeListMisordered = (
> >       qr{char\s+(?:un)?signed},
> >       qr{int\s+(?:(?:un)?signed\s+)?short\s},
> > @@ -2773,8 +2794,28 @@ sub process {
> >                       my $ucfirst_sign_off = ucfirst(lc($sign_off));
> >
> >                       if ($sign_off !~ /$signature_tags/) {
> > -                             WARN("BAD_SIGN_OFF",
> > -                                  "Non-standard signature: $sign_off\n"
> . $herecurr);
> > +                             my $suggested_signature = "";
> > +                             if
> (exists($standard_signature_fix{$sign_off})) {
> > +                                     $suggested_signature =
> $standard_signature_fix{$sign_off};
> > +                             }
> > +                             if ($suggested_signature eq "") {
> > +                                     WARN("BAD_SIGN_OFF",
> > +                                          "Non-standard signature:
> $sign_off\n" . $herecurr);
> > +                             }
> > +                             elsif ($suggested_signature eq "remove") {
> > +                                     if (WARN("BAD_SIGN_OFF",
> > +                                             "Non-standard signature:
> $sign_off. Please consider removing this signature tag.\n" . $herecurr) &&
> > +                                     $fix) {
> > +
>  fix_delete_line($fixlinenr, $rawline);
> > +                                     }
> > +                             }
> > +                             else {
> > +                                     if (WARN("BAD_SIGN_OFF",
> > +                                             "Non-standard signature:
> $sign_off. Please use '$suggested_signature' instead.\n" . $herecurr) &&
> > +                                     $fix) {
> > +                                             $fixed[$fixlinenr] =~
> s/$sign_off/$suggested_signature/;
> > +                                     }
> > +                             }
> >                       }
> >                       if (defined $space_before && $space_before ne "") {
> >                               if (WARN("BAD_SIGN_OFF",
> >
>
> Initial tests performed on patches found this fix to be working as
> expected.
>
> Thanks
> Aditya
>

[-- Attachment #1.2: Type: text/html, Size: 7527 bytes --]

[-- Attachment #2: Type: text/plain, Size: 201 bytes --]

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

Re: [Linux-kernel-mentees] [PATCH] checkpatch: add fix and improve warning msg for Non-standard signature

[Aditya]

On 21/11/20 1:53 am, Lukas Bulwahn wrote:
> On Fr., 20. Nov. 2020 at 21:03, Aditya <yashsri421@gmail.com> wrote:
> 
>> On 21/11/20 1:28 am, Aditya Srivastava wrote:
>>> Checkpatch.pl warns on non-standard signature styles.
>>>
>>> E.g., running checkpatch on commit 513f7f747e1c ("parisc: Fix vmap
>>> memory leak in ioremap()/iounmap()") reports this warning:
>>>
>>> WARNING: Non-standard signature: Noticed-by:
>>> Noticed-by: Sven Schnelle <svens@stackframe.org>
>>>
> 
> 
> This example really does not tell anyone much.
> 
> Replace it with a summary from your evaluation.
> 
Okay

> 
>>> Provide a fix by:
>>> 1) replacing the non-standard signature with its standard equivalent
>>> 2) removing the signature if it is not required
>>>
>>> Also, improve warning messages correspondingly, providing users
>>> suggestions to either replace or remove the signature
>>>
> 
> 
> Looks good.
> 
> 
> 
>>> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
>>> ---
>>>  scripts/checkpatch.pl | 45 +++++++++++++++++++++++++++++++++++++++++--
>>>  1 file changed, 43 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>>> index fdfd5ec09be6..23a21dc2c29a 100755
>>> --- a/scripts/checkpatch.pl
>>> +++ b/scripts/checkpatch.pl
>>> @@ -506,6 +506,27 @@ our $signature_tags = qr{(?xi:
>>>       Cc:
>>>  )};
>>>
>>> +our %standard_signature_fix = (
>>> +     "Requested-by:" => "Suggested-by:",
>>> +     "Co-authored-by:" => "Co-developed-by:",
>>> +     "Analyzed-by:" => "Co-developed-by:",
>>> +     "Analysed-by:" => "Co-developed-by:",
>>> +     "Improvements-by:" => "Co-developed-by:",
>>> +     "Noticed-by:" => "Reported-by:",
>>> +     "Inspired-by:" => "Suggested-by:",
>>> +     "Verified-by:" => "Tested-by:",
>>> +     "Okay-ished-by:" => "Acked-by:",
>>> +     "Acked-for-MFD-by:" => "Acked-by:",
>>> +     "Reviewed-off-by:" => "Reviewed-by:",
>>> +     "Proposed-by:" => "Suggested-by:",
>>> +     "Fixed-by:" => "Co-developed-by:",
>>> +     "Pointed-out-by:" => "Suggested-by:",
>>> +     "Pointed-at-by:" => "Suggested-by:",
>>> +     "Suggestions-by:" => "Suggested-by:",
>>> +     "Generated-by:" => "remove",
>>> +     "Celebrated-by:" => "remove",
>>> +);
>>> +
> 
> 
> How did create this list? I thought we looked at 30 cases...
> 
> 
Yes, correct. Others are the cases which required discussion like:
"Originally-by" and its variants (including 'Based-on-patch-by', etc),
"Bisected-by", "Diagnosed-by", "Root-caused-by".

Thanks
Aditya

>>>  our @typeListMisordered = (
>>>       qr{char\s+(?:un)?signed},
>>>       qr{int\s+(?:(?:un)?signed\s+)?short\s},
>>> @@ -2773,8 +2794,28 @@ sub process {
>>>                       my $ucfirst_sign_off = ucfirst(lc($sign_off));
>>>
>>>                       if ($sign_off !~ /$signature_tags/) {
>>> -                             WARN("BAD_SIGN_OFF",
>>> -                                  "Non-standard signature: $sign_off\n"
>> . $herecurr);
>>> +                             my $suggested_signature = "";
>>> +                             if
>> (exists($standard_signature_fix{$sign_off})) {
>>> +                                     $suggested_signature =
>> $standard_signature_fix{$sign_off};
>>> +                             }
>>> +                             if ($suggested_signature eq "") {
>>> +                                     WARN("BAD_SIGN_OFF",
>>> +                                          "Non-standard signature:
>> $sign_off\n" . $herecurr);
>>> +                             }
>>> +                             elsif ($suggested_signature eq "remove") {
>>> +                                     if (WARN("BAD_SIGN_OFF",
>>> +                                             "Non-standard signature:
>> $sign_off. Please consider removing this signature tag.\n" . $herecurr) &&
>>> +                                     $fix) {
>>> +
>>  fix_delete_line($fixlinenr, $rawline);
>>> +                                     }
>>> +                             }
>>> +                             else {
>>> +                                     if (WARN("BAD_SIGN_OFF",
>>> +                                             "Non-standard signature:
>> $sign_off. Please use '$suggested_signature' instead.\n" . $herecurr) &&
>>> +                                     $fix) {
>>> +                                             $fixed[$fixlinenr] =~
>> s/$sign_off/$suggested_signature/;
>>> +                                     }
>>> +                             }
>>>                       }
>>>                       if (defined $space_before && $space_before ne "") {
>>>                               if (WARN("BAD_SIGN_OFF",
>>>
>>
>> Initial tests performed on patches found this fix to be working as
>> expected.
>>
>> Thanks
>> Aditya
>>
> 

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

[Linux-kernel-mentees] [PATCH v2] checkpatch: add fix and improve warning msg for Non-standard signature

[Aditya Srivastava]

Checkpatch.pl warns on non-standard signature styles.

This warning usually occurs because of incorrect use of signature tags,
e.g. tags such as 'Co-authored-by', which although seem correct, is not
a standard signature. Its standard equivalent is 'Co-developed-by'.

An evaluation over v4.13..v5.8 found 'Co-authored-by' tag used 43 times

Similarly, 'Requested-by'(used 48 times) has its equivalent as
'Suggested-by', and so on.

Provide a fix by:
1) replacing the non-standard signature with its standard equivalent
2) removing the signature if it is not required

Also, improve warning messages correspondingly, providing users
suggestions to either replace or remove the signature

Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
---
changes in v2: replace commit specific example with brief evaluation

 scripts/checkpatch.pl | 45 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 43 insertions(+), 2 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index fdfd5ec09be6..23a21dc2c29a 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -506,6 +506,27 @@ our $signature_tags = qr{(?xi:
 	Cc:
 )};
 
+our %standard_signature_fix = (
+	"Requested-by:" => "Suggested-by:",
+	"Co-authored-by:" => "Co-developed-by:",
+	"Analyzed-by:" => "Co-developed-by:",
+	"Analysed-by:" => "Co-developed-by:",
+	"Improvements-by:" => "Co-developed-by:",
+	"Noticed-by:" => "Reported-by:",
+	"Inspired-by:" => "Suggested-by:",
+	"Verified-by:" => "Tested-by:",
+	"Okay-ished-by:" => "Acked-by:",
+	"Acked-for-MFD-by:" => "Acked-by:",
+	"Reviewed-off-by:" => "Reviewed-by:",
+	"Proposed-by:" => "Suggested-by:",
+	"Fixed-by:" => "Co-developed-by:",
+	"Pointed-out-by:" => "Suggested-by:",
+	"Pointed-at-by:" => "Suggested-by:",
+	"Suggestions-by:" => "Suggested-by:",
+	"Generated-by:" => "remove",
+	"Celebrated-by:" => "remove",
+);
+
 our @typeListMisordered = (
 	qr{char\s+(?:un)?signed},
 	qr{int\s+(?:(?:un)?signed\s+)?short\s},
@@ -2773,8 +2794,28 @@ sub process {
 			my $ucfirst_sign_off = ucfirst(lc($sign_off));
 
 			if ($sign_off !~ /$signature_tags/) {
-				WARN("BAD_SIGN_OFF",
-				     "Non-standard signature: $sign_off\n" . $herecurr);
+				my $suggested_signature = "";
+				if (exists($standard_signature_fix{$sign_off})) {
+					$suggested_signature = $standard_signature_fix{$sign_off};
+				}
+				if ($suggested_signature eq "") {
+					WARN("BAD_SIGN_OFF",
+					     "Non-standard signature: $sign_off\n" . $herecurr);
+				}
+				elsif ($suggested_signature eq "remove") {
+					if (WARN("BAD_SIGN_OFF",
+						"Non-standard signature: $sign_off. Please consider removing this signature tag.\n" . $herecurr) &&
+					$fix) {
+						fix_delete_line($fixlinenr, $rawline);
+					}
+				}
+				else {
+					if (WARN("BAD_SIGN_OFF",
+						"Non-standard signature: $sign_off. Please use '$suggested_signature' instead.\n" . $herecurr) &&
+					$fix) {
+						$fixed[$fixlinenr] =~ s/$sign_off/$suggested_signature/;
+					}
+				}
 			}
 			if (defined $space_before && $space_before ne "") {
 				if (WARN("BAD_SIGN_OFF",
-- 
2.17.1

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

Re: [Linux-kernel-mentees] [PATCH v2] checkpatch: add fix and improve warning msg for Non-standard signature

[Lukas Bulwahn]

On Sat, Nov 21, 2020 at 5:58 AM Aditya Srivastava <yashsri421@gmail.com> wrote:
>
> Checkpatch.pl warns on non-standard signature styles.
>
> This warning usually occurs because of incorrect use of signature tags,
> e.g. tags such as 'Co-authored-by', which although seem correct, is not
> a standard signature. Its standard equivalent is 'Co-developed-by'.
>
> An evaluation over v4.13..v5.8 found 'Co-authored-by' tag used 43 times
>
> Similarly, 'Requested-by'(used 48 times) has its equivalent as
> 'Suggested-by', and so on.
>

Better but still an incomplete summary.
"and so on" is not good.

Consider a description that explains how you came to the conclusion.

Then consider presenting the count of non-standard signature tags from
v4.13..v5.8 is a simple structured form.

By the way, there is no limit on the length of a commit message as
long as you convey information in a dense form.

Also, consider to present the rationale for each replacement.
checkpatch shall explain to its user why it considers the replacement
appropriate.


Lukas

> Provide a fix by:
> 1) replacing the non-standard signature with its standard equivalent
> 2) removing the signature if it is not required
>
> Also, improve warning messages correspondingly, providing users
> suggestions to either replace or remove the signature
>
> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
> ---
> changes in v2: replace commit specific example with brief evaluation
>
>  scripts/checkpatch.pl | 45 +++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 43 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index fdfd5ec09be6..23a21dc2c29a 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -506,6 +506,27 @@ our $signature_tags = qr{(?xi:
>         Cc:
>  )};
>
> +our %standard_signature_fix = (
> +       "Requested-by:" => "Suggested-by:",
> +       "Co-authored-by:" => "Co-developed-by:",
> +       "Analyzed-by:" => "Co-developed-by:",
> +       "Analysed-by:" => "Co-developed-by:",
> +       "Improvements-by:" => "Co-developed-by:",
> +       "Noticed-by:" => "Reported-by:",
> +       "Inspired-by:" => "Suggested-by:",
> +       "Verified-by:" => "Tested-by:",
> +       "Okay-ished-by:" => "Acked-by:",
> +       "Acked-for-MFD-by:" => "Acked-by:",
> +       "Reviewed-off-by:" => "Reviewed-by:",
> +       "Proposed-by:" => "Suggested-by:",
> +       "Fixed-by:" => "Co-developed-by:",
> +       "Pointed-out-by:" => "Suggested-by:",
> +       "Pointed-at-by:" => "Suggested-by:",
> +       "Suggestions-by:" => "Suggested-by:",
> +       "Generated-by:" => "remove",
> +       "Celebrated-by:" => "remove",
> +);
> +
>  our @typeListMisordered = (
>         qr{char\s+(?:un)?signed},
>         qr{int\s+(?:(?:un)?signed\s+)?short\s},
> @@ -2773,8 +2794,28 @@ sub process {
>                         my $ucfirst_sign_off = ucfirst(lc($sign_off));
>
>                         if ($sign_off !~ /$signature_tags/) {
> -                               WARN("BAD_SIGN_OFF",
> -                                    "Non-standard signature: $sign_off\n" . $herecurr);
> +                               my $suggested_signature = "";
> +                               if (exists($standard_signature_fix{$sign_off})) {
> +                                       $suggested_signature = $standard_signature_fix{$sign_off};
> +                               }
> +                               if ($suggested_signature eq "") {
> +                                       WARN("BAD_SIGN_OFF",
> +                                            "Non-standard signature: $sign_off\n" . $herecurr);
> +                               }
> +                               elsif ($suggested_signature eq "remove") {
> +                                       if (WARN("BAD_SIGN_OFF",
> +                                               "Non-standard signature: $sign_off. Please consider removing this signature tag.\n" . $herecurr) &&
> +                                       $fix) {
> +                                               fix_delete_line($fixlinenr, $rawline);
> +                                       }
> +                               }
> +                               else {
> +                                       if (WARN("BAD_SIGN_OFF",
> +                                               "Non-standard signature: $sign_off. Please use '$suggested_signature' instead.\n" . $herecurr) &&
> +                                       $fix) {
> +                                               $fixed[$fixlinenr] =~ s/$sign_off/$suggested_signature/;
> +                                       }
> +                               }
>                         }
>                         if (defined $space_before && $space_before ne "") {
>                                 if (WARN("BAD_SIGN_OFF",
> --
> 2.17.1
>
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

[Linux-kernel-mentees] [PATCH v3] checkpatch: add fix and improve warning msg for Non-standard signature

[Aditya Srivastava]

Currently, checkpatch.pl warns for BAD_SIGN_OFF on non-standard signature
styles.

This warning occurs because of incorrect use of signature tags,
e.g. an evaluation on v4.13..v5.8 showed the use of following incorrect
signature tags, which may seem correct, but are not standard:

1) Requested-by (count: 48) => Suggested-by
Rationale: In an open-source project, there are no 'requests', just
'suggestions' to convince a maintainer to accept your patch

2) Co-authored-by (count: 43) => Co-developed-by
Rationale: Co-developed-by and Co-authored-by are synonyms

3) Analyzed-by (count: 22) / Analysed-by (count: 5) => Co-developed-by
Rationale: Analyzing is a part of Software Development, so
'Co-developed-by' is perfectly fine, even if contributor did not create
code

4) Improvements-by (count: 19) => Co-developed-by

5) Noticed-by (count: 11) => Reported-by

6) Inspired-by (count: 11) => Suggested-by

7) Verified-by (count: 8) => Tested-by
Rationale: Used by a single user. On reading mailing list, it seems
Tested-by might be a suitable alternative

8) Okay-ished-by (count: 8) => Acked-by
Rationale: Used by a single user. On reading mailing list, it seems
Acked-by must be suitable alternative

9) Acked-for-MFD-by (count: 6) => Acked-by

10) Reviewed-off-by (count: 5) => Reviewed-by

11) Proposed-by (count: 5) => Suggested-by
Rationale: On observing the mailing list, this tag is always used for a
maintainer. It seems that the changes might have been suggested by them
and the tag is used as acknowledgement for the same

12) Fixed-by (count: 3) => Co-developed-by
Rationale: Fixing bug is a part of Software Development, so
'Co-developed-by' is perfectly fine, even if contributor did not create
code

13) Pointed-out-by (count: 3) / Pointed-at-by (count: 2) => Suggested-by
Rationale: The tags are used for maintainers. It seems that the changes
might have been suggested by them and the tag is used as acknowledgement
for the same
E.g., Pointed-at-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

14) Suggestions-by (count: 3) => Suggested-by

15) Generated-by (count: 17) => remove the tag
On observing the mailing list, this tag is always used for quoting the
tool or script, which might have been used to generate the patch.
E.g. Generated-by: scripts/coccinelle/api/alloc/kzalloc-simple.cocci

16) Celebrated-by (count: 3) => remove the tag
This tag was used for only one commit. On observing mailing list, it seem
like the celebration for a particular patch and changes.

Provide a fix by:
1) replacing the non-standard signature with its standard equivalent
2) removing the signature if it is not required

Also, improve warning messages correspondingly, providing users
suggestions to either replace or remove the signature. Also provide
suitable rationale to the user for the suggestion made.

Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
---
changes in v2: replace commit specific example with brief evaluation

changes in v3: provide rationale to users for every signature tag suggestion;
modify commit message describing arrival to conclusion in a structured way

 scripts/checkpatch.pl | 101 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 99 insertions(+), 2 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index fdfd5ec09be6..575ff8efb0eb 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -506,6 +506,81 @@ our $signature_tags = qr{(?xi:
 	Cc:
 )};
 
+our %standard_signature_fix = (
+	"Requested-by:" => {
+		suggestion => "Suggested-by:",
+		rationale => "In an open-source project, there are no 'requests', just 'suggestions' to convince a maintainer to accept your patch",
+	},
+	"Co-authored-by:" => {
+		suggestion => "Co-developed-by:",
+		rationale => "Co-developed-by and Co-authored-by are synonyms",
+	},
+	"Analyzed-by:" => {
+		suggestion => "Co-developed-by:",
+		rationale => "Analyzing is a part of Software Development, so 'Co-developed-by' is perfectly fine, even if contributor did not create code",
+	},
+	"Analysed-by:" => {
+		suggestion => "Co-developed-by:",
+		rationale => "Analysing is a part of Software Development, so 'Co-developed-by' is perfectly fine, even if contributor did not create code",
+	},
+	"Improvements-by:" => {
+		suggestion => "Co-developed-by:",
+		rationale => "Performing improvements are a part of Software Developement, so 'Co-developed-by' is perfectly fine, even if contributor did not create code",
+	},
+	"Noticed-by:" => {
+		suggestion => "Reported-by:",
+		rationale => "Reported-by and Noticed-by are synonyms",
+	},
+	"Inspired-by:" => {
+		suggestion => "Suggested-by:",
+		rationale => "Suggested-by is the standard signature tag for acknowledging user for their suggestions",
+	},
+	"Verified-by:" => {
+		suggestion => "Tested-by:",
+		rationale => "Tested-by and Verified-by are synonyms",
+	},
+	"Okay-ished-by:" => {
+		suggestion => "Acked-by:",
+		rationale => "Acked-by is the standard signature tag for recording your approval",
+	},
+	"Acked-for-MFD-by:" => {
+		suggestion => "Acked-by:",
+		rationale => "Acked-by is the standard signature tag for recording your approval",
+	},
+	"Reviewed-off-by:" => {
+		suggestion => "Reviewed-by:",
+		rationale => "Reviewed-by is the standard signature tag for recording your approval",
+	},
+	"Proposed-by:" => {
+		suggestion => "Suggested-by:",
+		rationale => "Proposing changes is same as suggesting changes, so Suggested-by seems perfectly fine",
+	},
+	"Fixed-by:" => {
+		suggestion => "Co-developed-by:",
+		rationale => "Fixing bug is a part of Software Development, so 'Co-developed-by' is perfectly fine, even if contributor did not create code",
+	},
+	"Pointed-out-by:" => {
+		suggestion => "Suggested-by:",
+		rationale => "Pointing out certain changes is synonymous to suggesting changes, so Suggested-by seems perfectly fine",
+	},
+	"Pointed-at-by:" => {
+		suggestion => "Suggested-by:",
+		rationale => "Pointing at certain changes is synonymous to suggesting changes, so Suggested-by seems perfectly fine",
+	},
+	"Suggestions-by:" => {
+		suggestion => "Suggested-by:",
+		rationale => "Suggested-by is the standard signature tag for acknowledging user for their suggestions",
+	},
+	"Generated-by:" => {
+		suggestion => "remove",
+		rationale => "Signature tags are used to acknowledge users for their contributions. It is advised to describe about tools in commit description instead",
+	},
+	"Celebrated-by:" => {
+		suggestion => "remove",
+		rationale => "Signature tags are used to acknowledge users for their contributions. This tag may not be required at all",
+	},
+);
+
 our @typeListMisordered = (
 	qr{char\s+(?:un)?signed},
 	qr{int\s+(?:(?:un)?signed\s+)?short\s},
@@ -2773,8 +2848,30 @@ sub process {
 			my $ucfirst_sign_off = ucfirst(lc($sign_off));
 
 			if ($sign_off !~ /$signature_tags/) {
-				WARN("BAD_SIGN_OFF",
-				     "Non-standard signature: $sign_off\n" . $herecurr);
+				my $suggested_signature = "";
+				my $rationale = "";
+				if (exists($standard_signature_fix{$sign_off})) {
+					$suggested_signature = $standard_signature_fix{$sign_off}{'suggestion'};
+					$rationale = $standard_signature_fix{$sign_off}{'rationale'};
+				}
+				if ($suggested_signature eq "") {
+					WARN("BAD_SIGN_OFF",
+					     "Non-standard signature: $sign_off\n" . $herecurr);
+				}
+				elsif ($suggested_signature eq "remove") {
+					if (WARN("BAD_SIGN_OFF",
+						"Non-standard signature: $sign_off. Please consider removing this signature tag. $rationale\n" . $herecurr) &&
+					$fix) {
+						fix_delete_line($fixlinenr, $rawline);
+					}
+				}
+				else {
+					if (WARN("BAD_SIGN_OFF",
+						"Non-standard signature: $sign_off. Please use '$suggested_signature' instead. $rationale\n" . $herecurr) &&
+					$fix) {
+						$fixed[$fixlinenr] =~ s/$sign_off/$suggested_signature/;
+					}
+				}
 			}
 			if (defined $space_before && $space_before ne "") {
 				if (WARN("BAD_SIGN_OFF",
-- 
2.17.1

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

Re: [Linux-kernel-mentees] [PATCH v3] checkpatch: add fix and improve warning msg for Non-standard signature

[Lukas Bulwahn]

On Mon, Nov 23, 2020 at 1:21 PM Aditya Srivastava <yashsri421@gmail.com> wrote:
>
> Currently, checkpatch.pl warns for BAD_SIGN_OFF on non-standard signature
> styles.
>
> This warning occurs because of incorrect use of signature tags,
> e.g. an evaluation on v4.13..v5.8 showed the use of following incorrect
> signature tags, which may seem correct, but are not standard:
>
> 1) Requested-by (count: 48) => Suggested-by
> Rationale: In an open-source project, there are no 'requests', just
> 'suggestions' to convince a maintainer to accept your patch
>
> 2) Co-authored-by (count: 43) => Co-developed-by
> Rationale: Co-developed-by and Co-authored-by are synonyms
>
> 3) Analyzed-by (count: 22) / Analysed-by (count: 5) => Co-developed-by
> Rationale: Analyzing is a part of Software Development, so
> 'Co-developed-by' is perfectly fine, even if contributor did not create
> code
>
> 4) Improvements-by (count: 19) => Co-developed-by
>
> 5) Noticed-by (count: 11) => Reported-by
>
> 6) Inspired-by (count: 11) => Suggested-by
>
> 7) Verified-by (count: 8) => Tested-by
> Rationale: Used by a single user. On reading mailing list, it seems
> Tested-by might be a suitable alternative
>
> 8) Okay-ished-by (count: 8) => Acked-by
> Rationale: Used by a single user. On reading mailing list, it seems
> Acked-by must be suitable alternative
>
> 9) Acked-for-MFD-by (count: 6) => Acked-by
>
> 10) Reviewed-off-by (count: 5) => Reviewed-by
>
> 11) Proposed-by (count: 5) => Suggested-by
> Rationale: On observing the mailing list, this tag is always used for a
> maintainer. It seems that the changes might have been suggested by them
> and the tag is used as acknowledgement for the same
>
> 12) Fixed-by (count: 3) => Co-developed-by
> Rationale: Fixing bug is a part of Software Development, so
> 'Co-developed-by' is perfectly fine, even if contributor did not create
> code
>
> 13) Pointed-out-by (count: 3) / Pointed-at-by (count: 2) => Suggested-by
> Rationale: The tags are used for maintainers. It seems that the changes
> might have been suggested by them and the tag is used as acknowledgement
> for the same
> E.g., Pointed-at-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>
> 14) Suggestions-by (count: 3) => Suggested-by
>
> 15) Generated-by (count: 17) => remove the tag
> On observing the mailing list, this tag is always used for quoting the
> tool or script, which might have been used to generate the patch.
> E.g. Generated-by: scripts/coccinelle/api/alloc/kzalloc-simple.cocci
>
> 16) Celebrated-by (count: 3) => remove the tag
> This tag was used for only one commit. On observing mailing list, it seem
> like the celebration for a particular patch and changes.
>
> Provide a fix by:
> 1) replacing the non-standard signature with its standard equivalent
> 2) removing the signature if it is not required
>
> Also, improve warning messages correspondingly, providing users
> suggestions to either replace or remove the signature. Also provide
> suitable rationale to the user for the suggestion made.
>
> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>


Looks good to me. Let us propose this to Joe for review.

Also, you can already start working on the related feature for
providing fixes based on the edit distance.

With both features, we can probably fix all non-standard signatures...

Lukas

> ---
> changes in v2: replace commit specific example with brief evaluation
>
> changes in v3: provide rationale to users for every signature tag suggestion;
> modify commit message describing arrival to conclusion in a structured way
>
>  scripts/checkpatch.pl | 101 +++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 99 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index fdfd5ec09be6..575ff8efb0eb 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -506,6 +506,81 @@ our $signature_tags = qr{(?xi:
>         Cc:
>  )};
>
> +our %standard_signature_fix = (
> +       "Requested-by:" => {
> +               suggestion => "Suggested-by:",
> +               rationale => "In an open-source project, there are no 'requests', just 'suggestions' to convince a maintainer to accept your patch",
> +       },
> +       "Co-authored-by:" => {
> +               suggestion => "Co-developed-by:",
> +               rationale => "Co-developed-by and Co-authored-by are synonyms",
> +       },
> +       "Analyzed-by:" => {
> +               suggestion => "Co-developed-by:",
> +               rationale => "Analyzing is a part of Software Development, so 'Co-developed-by' is perfectly fine, even if contributor did not create code",
> +       },
> +       "Analysed-by:" => {
> +               suggestion => "Co-developed-by:",
> +               rationale => "Analysing is a part of Software Development, so 'Co-developed-by' is perfectly fine, even if contributor did not create code",
> +       },
> +       "Improvements-by:" => {
> +               suggestion => "Co-developed-by:",
> +               rationale => "Performing improvements are a part of Software Developement, so 'Co-developed-by' is perfectly fine, even if contributor did not create code",
> +       },
> +       "Noticed-by:" => {
> +               suggestion => "Reported-by:",
> +               rationale => "Reported-by and Noticed-by are synonyms",
> +       },
> +       "Inspired-by:" => {
> +               suggestion => "Suggested-by:",
> +               rationale => "Suggested-by is the standard signature tag for acknowledging user for their suggestions",
> +       },
> +       "Verified-by:" => {
> +               suggestion => "Tested-by:",
> +               rationale => "Tested-by and Verified-by are synonyms",
> +       },
> +       "Okay-ished-by:" => {
> +               suggestion => "Acked-by:",
> +               rationale => "Acked-by is the standard signature tag for recording your approval",
> +       },
> +       "Acked-for-MFD-by:" => {
> +               suggestion => "Acked-by:",
> +               rationale => "Acked-by is the standard signature tag for recording your approval",
> +       },
> +       "Reviewed-off-by:" => {
> +               suggestion => "Reviewed-by:",
> +               rationale => "Reviewed-by is the standard signature tag for recording your approval",
> +       },
> +       "Proposed-by:" => {
> +               suggestion => "Suggested-by:",
> +               rationale => "Proposing changes is same as suggesting changes, so Suggested-by seems perfectly fine",
> +       },
> +       "Fixed-by:" => {
> +               suggestion => "Co-developed-by:",
> +               rationale => "Fixing bug is a part of Software Development, so 'Co-developed-by' is perfectly fine, even if contributor did not create code",
> +       },
> +       "Pointed-out-by:" => {
> +               suggestion => "Suggested-by:",
> +               rationale => "Pointing out certain changes is synonymous to suggesting changes, so Suggested-by seems perfectly fine",
> +       },
> +       "Pointed-at-by:" => {
> +               suggestion => "Suggested-by:",
> +               rationale => "Pointing at certain changes is synonymous to suggesting changes, so Suggested-by seems perfectly fine",
> +       },
> +       "Suggestions-by:" => {
> +               suggestion => "Suggested-by:",
> +               rationale => "Suggested-by is the standard signature tag for acknowledging user for their suggestions",
> +       },
> +       "Generated-by:" => {
> +               suggestion => "remove",
> +               rationale => "Signature tags are used to acknowledge users for their contributions. It is advised to describe about tools in commit description instead",
> +       },
> +       "Celebrated-by:" => {
> +               suggestion => "remove",
> +               rationale => "Signature tags are used to acknowledge users for their contributions. This tag may not be required at all",
> +       },
> +);
> +
>  our @typeListMisordered = (
>         qr{char\s+(?:un)?signed},
>         qr{int\s+(?:(?:un)?signed\s+)?short\s},
> @@ -2773,8 +2848,30 @@ sub process {
>                         my $ucfirst_sign_off = ucfirst(lc($sign_off));
>
>                         if ($sign_off !~ /$signature_tags/) {
> -                               WARN("BAD_SIGN_OFF",
> -                                    "Non-standard signature: $sign_off\n" . $herecurr);
> +                               my $suggested_signature = "";
> +                               my $rationale = "";
> +                               if (exists($standard_signature_fix{$sign_off})) {
> +                                       $suggested_signature = $standard_signature_fix{$sign_off}{'suggestion'};
> +                                       $rationale = $standard_signature_fix{$sign_off}{'rationale'};
> +                               }
> +                               if ($suggested_signature eq "") {
> +                                       WARN("BAD_SIGN_OFF",
> +                                            "Non-standard signature: $sign_off\n" . $herecurr);
> +                               }
> +                               elsif ($suggested_signature eq "remove") {
> +                                       if (WARN("BAD_SIGN_OFF",
> +                                               "Non-standard signature: $sign_off. Please consider removing this signature tag. $rationale\n" . $herecurr) &&
> +                                       $fix) {
> +                                               fix_delete_line($fixlinenr, $rawline);
> +                                       }
> +                               }
> +                               else {
> +                                       if (WARN("BAD_SIGN_OFF",
> +                                               "Non-standard signature: $sign_off. Please use '$suggested_signature' instead. $rationale\n" . $herecurr) &&
> +                                       $fix) {
> +                                               $fixed[$fixlinenr] =~ s/$sign_off/$suggested_signature/;
> +                                       }
> +                               }
>                         }
>                         if (defined $space_before && $space_before ne "") {
>                                 if (WARN("BAD_SIGN_OFF",
> --
> 2.17.1
>
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [Linux-kernel-mentees] [PATCH v3] checkpatch: add fix and improve warning msg for Non-standard signature

[Aditya]

On 23/11/20 6:39 pm, Lukas Bulwahn wrote:
> On Mon, Nov 23, 2020 at 1:21 PM Aditya Srivastava <yashsri421@gmail.com> wrote:
>>
>> Currently, checkpatch.pl warns for BAD_SIGN_OFF on non-standard signature
>> styles.
>>
>> This warning occurs because of incorrect use of signature tags,
>> e.g. an evaluation on v4.13..v5.8 showed the use of following incorrect
>> signature tags, which may seem correct, but are not standard:
>>
>> 1) Requested-by (count: 48) => Suggested-by
>> Rationale: In an open-source project, there are no 'requests', just
>> 'suggestions' to convince a maintainer to accept your patch
>>
>> 2) Co-authored-by (count: 43) => Co-developed-by
>> Rationale: Co-developed-by and Co-authored-by are synonyms
>>
>> 3) Analyzed-by (count: 22) / Analysed-by (count: 5) => Co-developed-by
>> Rationale: Analyzing is a part of Software Development, so
>> 'Co-developed-by' is perfectly fine, even if contributor did not create
>> code
>>
>> 4) Improvements-by (count: 19) => Co-developed-by
>>
>> 5) Noticed-by (count: 11) => Reported-by
>>
>> 6) Inspired-by (count: 11) => Suggested-by
>>
>> 7) Verified-by (count: 8) => Tested-by
>> Rationale: Used by a single user. On reading mailing list, it seems
>> Tested-by might be a suitable alternative
>>
>> 8) Okay-ished-by (count: 8) => Acked-by
>> Rationale: Used by a single user. On reading mailing list, it seems
>> Acked-by must be suitable alternative
>>
>> 9) Acked-for-MFD-by (count: 6) => Acked-by
>>
>> 10) Reviewed-off-by (count: 5) => Reviewed-by
>>
>> 11) Proposed-by (count: 5) => Suggested-by
>> Rationale: On observing the mailing list, this tag is always used for a
>> maintainer. It seems that the changes might have been suggested by them
>> and the tag is used as acknowledgement for the same
>>
>> 12) Fixed-by (count: 3) => Co-developed-by
>> Rationale: Fixing bug is a part of Software Development, so
>> 'Co-developed-by' is perfectly fine, even if contributor did not create
>> code
>>
>> 13) Pointed-out-by (count: 3) / Pointed-at-by (count: 2) => Suggested-by
>> Rationale: The tags are used for maintainers. It seems that the changes
>> might have been suggested by them and the tag is used as acknowledgement
>> for the same
>> E.g., Pointed-at-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>
>> 14) Suggestions-by (count: 3) => Suggested-by
>>
>> 15) Generated-by (count: 17) => remove the tag
>> On observing the mailing list, this tag is always used for quoting the
>> tool or script, which might have been used to generate the patch.
>> E.g. Generated-by: scripts/coccinelle/api/alloc/kzalloc-simple.cocci
>>
>> 16) Celebrated-by (count: 3) => remove the tag
>> This tag was used for only one commit. On observing mailing list, it seem
>> like the celebration for a particular patch and changes.
>>
>> Provide a fix by:
>> 1) replacing the non-standard signature with its standard equivalent
>> 2) removing the signature if it is not required
>>
>> Also, improve warning messages correspondingly, providing users
>> suggestions to either replace or remove the signature. Also provide
>> suitable rationale to the user for the suggestion made.
>>
>> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
> 
> 
> Looks good to me. Let us propose this to Joe for review.
> 
Sent it. We probably still need to discuss about the remaining signatures:
1) "Debugged-by", 61
2) "Originally-by", 39
3) "Bisected-by", 20
4) "Diagnosed-by", 11
5) "Original-patch-by", 11
6) "Based-on-patch-by", 7
7) "Based-on-a-patch-by", 8
8) "Root-caused-by", 6
9) "Original-by", 6
10) "Based-on-patches-by", 5
11) "Based-on-work-by", 5

Should I send this list as a follow-up mail?

> Also, you can already start working on the related feature for
> providing fixes based on the edit distance.
> 
Okay.

Thanks
Aditya
> With both features, we can probably fix all non-standard signatures...
> 
> Lukas
> 
>> ---
>> changes in v2: replace commit specific example with brief evaluation
>>
>> changes in v3: provide rationale to users for every signature tag suggestion;
>> modify commit message describing arrival to conclusion in a structured way
>>
>>  scripts/checkpatch.pl | 101 +++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 99 insertions(+), 2 deletions(-)
>>
>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>> index fdfd5ec09be6..575ff8efb0eb 100755
>> --- a/scripts/checkpatch.pl
>> +++ b/scripts/checkpatch.pl
>> @@ -506,6 +506,81 @@ our $signature_tags = qr{(?xi:
>>         Cc:
>>  )};
>>
>> +our %standard_signature_fix = (
>> +       "Requested-by:" => {
>> +               suggestion => "Suggested-by:",
>> +               rationale => "In an open-source project, there are no 'requests', just 'suggestions' to convince a maintainer to accept your patch",
>> +       },
>> +       "Co-authored-by:" => {
>> +               suggestion => "Co-developed-by:",
>> +               rationale => "Co-developed-by and Co-authored-by are synonyms",
>> +       },
>> +       "Analyzed-by:" => {
>> +               suggestion => "Co-developed-by:",
>> +               rationale => "Analyzing is a part of Software Development, so 'Co-developed-by' is perfectly fine, even if contributor did not create code",
>> +       },
>> +       "Analysed-by:" => {
>> +               suggestion => "Co-developed-by:",
>> +               rationale => "Analysing is a part of Software Development, so 'Co-developed-by' is perfectly fine, even if contributor did not create code",
>> +       },
>> +       "Improvements-by:" => {
>> +               suggestion => "Co-developed-by:",
>> +               rationale => "Performing improvements are a part of Software Developement, so 'Co-developed-by' is perfectly fine, even if contributor did not create code",
>> +       },
>> +       "Noticed-by:" => {
>> +               suggestion => "Reported-by:",
>> +               rationale => "Reported-by and Noticed-by are synonyms",
>> +       },
>> +       "Inspired-by:" => {
>> +               suggestion => "Suggested-by:",
>> +               rationale => "Suggested-by is the standard signature tag for acknowledging user for their suggestions",
>> +       },
>> +       "Verified-by:" => {
>> +               suggestion => "Tested-by:",
>> +               rationale => "Tested-by and Verified-by are synonyms",
>> +       },
>> +       "Okay-ished-by:" => {
>> +               suggestion => "Acked-by:",
>> +               rationale => "Acked-by is the standard signature tag for recording your approval",
>> +       },
>> +       "Acked-for-MFD-by:" => {
>> +               suggestion => "Acked-by:",
>> +               rationale => "Acked-by is the standard signature tag for recording your approval",
>> +       },
>> +       "Reviewed-off-by:" => {
>> +               suggestion => "Reviewed-by:",
>> +               rationale => "Reviewed-by is the standard signature tag for recording your approval",
>> +       },
>> +       "Proposed-by:" => {
>> +               suggestion => "Suggested-by:",
>> +               rationale => "Proposing changes is same as suggesting changes, so Suggested-by seems perfectly fine",
>> +       },
>> +       "Fixed-by:" => {
>> +               suggestion => "Co-developed-by:",
>> +               rationale => "Fixing bug is a part of Software Development, so 'Co-developed-by' is perfectly fine, even if contributor did not create code",
>> +       },
>> +       "Pointed-out-by:" => {
>> +               suggestion => "Suggested-by:",
>> +               rationale => "Pointing out certain changes is synonymous to suggesting changes, so Suggested-by seems perfectly fine",
>> +       },
>> +       "Pointed-at-by:" => {
>> +               suggestion => "Suggested-by:",
>> +               rationale => "Pointing at certain changes is synonymous to suggesting changes, so Suggested-by seems perfectly fine",
>> +       },
>> +       "Suggestions-by:" => {
>> +               suggestion => "Suggested-by:",
>> +               rationale => "Suggested-by is the standard signature tag for acknowledging user for their suggestions",
>> +       },
>> +       "Generated-by:" => {
>> +               suggestion => "remove",
>> +               rationale => "Signature tags are used to acknowledge users for their contributions. It is advised to describe about tools in commit description instead",
>> +       },
>> +       "Celebrated-by:" => {
>> +               suggestion => "remove",
>> +               rationale => "Signature tags are used to acknowledge users for their contributions. This tag may not be required at all",
>> +       },
>> +);
>> +
>>  our @typeListMisordered = (
>>         qr{char\s+(?:un)?signed},
>>         qr{int\s+(?:(?:un)?signed\s+)?short\s},
>> @@ -2773,8 +2848,30 @@ sub process {
>>                         my $ucfirst_sign_off = ucfirst(lc($sign_off));
>>
>>                         if ($sign_off !~ /$signature_tags/) {
>> -                               WARN("BAD_SIGN_OFF",
>> -                                    "Non-standard signature: $sign_off\n" . $herecurr);
>> +                               my $suggested_signature = "";
>> +                               my $rationale = "";
>> +                               if (exists($standard_signature_fix{$sign_off})) {
>> +                                       $suggested_signature = $standard_signature_fix{$sign_off}{'suggestion'};
>> +                                       $rationale = $standard_signature_fix{$sign_off}{'rationale'};
>> +                               }
>> +                               if ($suggested_signature eq "") {
>> +                                       WARN("BAD_SIGN_OFF",
>> +                                            "Non-standard signature: $sign_off\n" . $herecurr);
>> +                               }
>> +                               elsif ($suggested_signature eq "remove") {
>> +                                       if (WARN("BAD_SIGN_OFF",
>> +                                               "Non-standard signature: $sign_off. Please consider removing this signature tag. $rationale\n" . $herecurr) &&
>> +                                       $fix) {
>> +                                               fix_delete_line($fixlinenr, $rawline);
>> +                                       }
>> +                               }
>> +                               else {
>> +                                       if (WARN("BAD_SIGN_OFF",
>> +                                               "Non-standard signature: $sign_off. Please use '$suggested_signature' instead. $rationale\n" . $herecurr) &&
>> +                                       $fix) {
>> +                                               $fixed[$fixlinenr] =~ s/$sign_off/$suggested_signature/;
>> +                                       }
>> +                               }
>>                         }
>>                         if (defined $space_before && $space_before ne "") {
>>                                 if (WARN("BAD_SIGN_OFF",
>> --
>> 2.17.1
>>

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

Re: [Linux-kernel-mentees] [PATCH v3] checkpatch: add fix and improve warning msg for Non-standard signature

[Lukas Bulwahn]

On Mon, Nov 23, 2020 at 4:16 PM Aditya <yashsri421@gmail.com> wrote:
>
> On 23/11/20 6:39 pm, Lukas Bulwahn wrote:
> > On Mon, Nov 23, 2020 at 1:21 PM Aditya Srivastava <yashsri421@gmail.com> wrote:
> >>
> >> Currently, checkpatch.pl warns for BAD_SIGN_OFF on non-standard signature
> >> styles.
> >>
> >> This warning occurs because of incorrect use of signature tags,
> >> e.g. an evaluation on v4.13..v5.8 showed the use of following incorrect
> >> signature tags, which may seem correct, but are not standard:
> >>
> >> 1) Requested-by (count: 48) => Suggested-by
> >> Rationale: In an open-source project, there are no 'requests', just
> >> 'suggestions' to convince a maintainer to accept your patch
> >>
> >> 2) Co-authored-by (count: 43) => Co-developed-by
> >> Rationale: Co-developed-by and Co-authored-by are synonyms
> >>
> >> 3) Analyzed-by (count: 22) / Analysed-by (count: 5) => Co-developed-by
> >> Rationale: Analyzing is a part of Software Development, so
> >> 'Co-developed-by' is perfectly fine, even if contributor did not create
> >> code
> >>
> >> 4) Improvements-by (count: 19) => Co-developed-by
> >>
> >> 5) Noticed-by (count: 11) => Reported-by
> >>
> >> 6) Inspired-by (count: 11) => Suggested-by
> >>
> >> 7) Verified-by (count: 8) => Tested-by
> >> Rationale: Used by a single user. On reading mailing list, it seems
> >> Tested-by might be a suitable alternative
> >>
> >> 8) Okay-ished-by (count: 8) => Acked-by
> >> Rationale: Used by a single user. On reading mailing list, it seems
> >> Acked-by must be suitable alternative
> >>
> >> 9) Acked-for-MFD-by (count: 6) => Acked-by
> >>
> >> 10) Reviewed-off-by (count: 5) => Reviewed-by
> >>
> >> 11) Proposed-by (count: 5) => Suggested-by
> >> Rationale: On observing the mailing list, this tag is always used for a
> >> maintainer. It seems that the changes might have been suggested by them
> >> and the tag is used as acknowledgement for the same
> >>
> >> 12) Fixed-by (count: 3) => Co-developed-by
> >> Rationale: Fixing bug is a part of Software Development, so
> >> 'Co-developed-by' is perfectly fine, even if contributor did not create
> >> code
> >>
> >> 13) Pointed-out-by (count: 3) / Pointed-at-by (count: 2) => Suggested-by
> >> Rationale: The tags are used for maintainers. It seems that the changes
> >> might have been suggested by them and the tag is used as acknowledgement
> >> for the same
> >> E.g., Pointed-at-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >>
> >> 14) Suggestions-by (count: 3) => Suggested-by
> >>
> >> 15) Generated-by (count: 17) => remove the tag
> >> On observing the mailing list, this tag is always used for quoting the
> >> tool or script, which might have been used to generate the patch.
> >> E.g. Generated-by: scripts/coccinelle/api/alloc/kzalloc-simple.cocci
> >>
> >> 16) Celebrated-by (count: 3) => remove the tag
> >> This tag was used for only one commit. On observing mailing list, it seem
> >> like the celebration for a particular patch and changes.
> >>
> >> Provide a fix by:
> >> 1) replacing the non-standard signature with its standard equivalent
> >> 2) removing the signature if it is not required
> >>
> >> Also, improve warning messages correspondingly, providing users
> >> suggestions to either replace or remove the signature. Also provide
> >> suitable rationale to the user for the suggestion made.
> >>
> >> Signed-off-by: Aditya Srivastava <yashsri421@gmail.com>
> >
> >
> > Looks good to me. Let us propose this to Joe for review.
> >
> Sent it. We probably still need to discuss about the remaining signatures:
> 1) "Debugged-by", 61
> 2) "Originally-by", 39
> 3) "Bisected-by", 20
> 4) "Diagnosed-by", 11
> 5) "Original-patch-by", 11
> 6) "Based-on-patch-by", 7
> 7) "Based-on-a-patch-by", 8
> 8) "Root-caused-by", 6
> 9) "Original-by", 6
> 10) "Based-on-patches-by", 5
> 11) "Based-on-work-by", 5
>
> Should I send this list as a follow-up mail?
>

Let us give Joe time to review the patch first; then if accepted, we
can continue the discussion on those tags above; you can already
propose a solution you have in mind and think about where the
documentation, checkpatch etc. needs to be adjusted.

> > Also, you can already start working on the related feature for
> > providing fixes based on the edit distance.
> >
> Okay.
>
> Thanks
> Aditya
> > With both features, we can probably fix all non-standard signatures...
> >
> > Lukas
> >
> >> ---
> >> changes in v2: replace commit specific example with brief evaluation
> >>
> >> changes in v3: provide rationale to users for every signature tag suggestion;
> >> modify commit message describing arrival to conclusion in a structured way
> >>
> >>  scripts/checkpatch.pl | 101 +++++++++++++++++++++++++++++++++++++++++-
> >>  1 file changed, 99 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> >> index fdfd5ec09be6..575ff8efb0eb 100755
> >> --- a/scripts/checkpatch.pl
> >> +++ b/scripts/checkpatch.pl
> >> @@ -506,6 +506,81 @@ our $signature_tags = qr{(?xi:
> >>         Cc:
> >>  )};
> >>
> >> +our %standard_signature_fix = (
> >> +       "Requested-by:" => {
> >> +               suggestion => "Suggested-by:",
> >> +               rationale => "In an open-source project, there are no 'requests', just 'suggestions' to convince a maintainer to accept your patch",
> >> +       },
> >> +       "Co-authored-by:" => {
> >> +               suggestion => "Co-developed-by:",
> >> +               rationale => "Co-developed-by and Co-authored-by are synonyms",
> >> +       },
> >> +       "Analyzed-by:" => {
> >> +               suggestion => "Co-developed-by:",
> >> +               rationale => "Analyzing is a part of Software Development, so 'Co-developed-by' is perfectly fine, even if contributor did not create code",
> >> +       },
> >> +       "Analysed-by:" => {
> >> +               suggestion => "Co-developed-by:",
> >> +               rationale => "Analysing is a part of Software Development, so 'Co-developed-by' is perfectly fine, even if contributor did not create code",
> >> +       },
> >> +       "Improvements-by:" => {
> >> +               suggestion => "Co-developed-by:",
> >> +               rationale => "Performing improvements are a part of Software Developement, so 'Co-developed-by' is perfectly fine, even if contributor did not create code",
> >> +       },
> >> +       "Noticed-by:" => {
> >> +               suggestion => "Reported-by:",
> >> +               rationale => "Reported-by and Noticed-by are synonyms",
> >> +       },
> >> +       "Inspired-by:" => {
> >> +               suggestion => "Suggested-by:",
> >> +               rationale => "Suggested-by is the standard signature tag for acknowledging user for their suggestions",
> >> +       },
> >> +       "Verified-by:" => {
> >> +               suggestion => "Tested-by:",
> >> +               rationale => "Tested-by and Verified-by are synonyms",
> >> +       },
> >> +       "Okay-ished-by:" => {
> >> +               suggestion => "Acked-by:",
> >> +               rationale => "Acked-by is the standard signature tag for recording your approval",
> >> +       },
> >> +       "Acked-for-MFD-by:" => {
> >> +               suggestion => "Acked-by:",
> >> +               rationale => "Acked-by is the standard signature tag for recording your approval",
> >> +       },
> >> +       "Reviewed-off-by:" => {
> >> +               suggestion => "Reviewed-by:",
> >> +               rationale => "Reviewed-by is the standard signature tag for recording your approval",
> >> +       },
> >> +       "Proposed-by:" => {
> >> +               suggestion => "Suggested-by:",
> >> +               rationale => "Proposing changes is same as suggesting changes, so Suggested-by seems perfectly fine",
> >> +       },
> >> +       "Fixed-by:" => {
> >> +               suggestion => "Co-developed-by:",
> >> +               rationale => "Fixing bug is a part of Software Development, so 'Co-developed-by' is perfectly fine, even if contributor did not create code",
> >> +       },
> >> +       "Pointed-out-by:" => {
> >> +               suggestion => "Suggested-by:",
> >> +               rationale => "Pointing out certain changes is synonymous to suggesting changes, so Suggested-by seems perfectly fine",
> >> +       },
> >> +       "Pointed-at-by:" => {
> >> +               suggestion => "Suggested-by:",
> >> +               rationale => "Pointing at certain changes is synonymous to suggesting changes, so Suggested-by seems perfectly fine",
> >> +       },
> >> +       "Suggestions-by:" => {
> >> +               suggestion => "Suggested-by:",
> >> +               rationale => "Suggested-by is the standard signature tag for acknowledging user for their suggestions",
> >> +       },
> >> +       "Generated-by:" => {
> >> +               suggestion => "remove",
> >> +               rationale => "Signature tags are used to acknowledge users for their contributions. It is advised to describe about tools in commit description instead",
> >> +       },
> >> +       "Celebrated-by:" => {
> >> +               suggestion => "remove",
> >> +               rationale => "Signature tags are used to acknowledge users for their contributions. This tag may not be required at all",
> >> +       },
> >> +);
> >> +
> >>  our @typeListMisordered = (
> >>         qr{char\s+(?:un)?signed},
> >>         qr{int\s+(?:(?:un)?signed\s+)?short\s},
> >> @@ -2773,8 +2848,30 @@ sub process {
> >>                         my $ucfirst_sign_off = ucfirst(lc($sign_off));
> >>
> >>                         if ($sign_off !~ /$signature_tags/) {
> >> -                               WARN("BAD_SIGN_OFF",
> >> -                                    "Non-standard signature: $sign_off\n" . $herecurr);
> >> +                               my $suggested_signature = "";
> >> +                               my $rationale = "";
> >> +                               if (exists($standard_signature_fix{$sign_off})) {
> >> +                                       $suggested_signature = $standard_signature_fix{$sign_off}{'suggestion'};
> >> +                                       $rationale = $standard_signature_fix{$sign_off}{'rationale'};
> >> +                               }
> >> +                               if ($suggested_signature eq "") {
> >> +                                       WARN("BAD_SIGN_OFF",
> >> +                                            "Non-standard signature: $sign_off\n" . $herecurr);
> >> +                               }
> >> +                               elsif ($suggested_signature eq "remove") {
> >> +                                       if (WARN("BAD_SIGN_OFF",
> >> +                                               "Non-standard signature: $sign_off. Please consider removing this signature tag. $rationale\n" . $herecurr) &&
> >> +                                       $fix) {
> >> +                                               fix_delete_line($fixlinenr, $rawline);
> >> +                                       }
> >> +                               }
> >> +                               else {
> >> +                                       if (WARN("BAD_SIGN_OFF",
> >> +                                               "Non-standard signature: $sign_off. Please use '$suggested_signature' instead. $rationale\n" . $herecurr) &&
> >> +                                       $fix) {
> >> +                                               $fixed[$fixlinenr] =~ s/$sign_off/$suggested_signature/;
> >> +                                       }
> >> +                               }
> >>                         }
> >>                         if (defined $space_before && $space_before ne "") {
> >>                                 if (WARN("BAD_SIGN_OFF",
> >> --
> >> 2.17.1
> >>
>
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees