[Linux-kernel-mentees] [PATCH] checkpatch: add fix option for MISSING_SIGN_OFF

[Aditya]

On 9/11/20 11:20 pm, Lukas Bulwahn wrote:
> On Mon, Nov 9, 2020 at 5:35 PM Aditya <yashsri421 at gmail.com> wrote:
>>
>> On 9/11/20 8:15 pm, Lukas Bulwahn wrote:
>>> On Mon, Nov 9, 2020 at 3:33 PM Aditya <yashsri421 at gmail.com> wrote:
>>>>
>>>> On 9/11/20 7:10 pm, Lukas Bulwahn wrote:
>>>>> On Mo., 9. Nov. 2020 at 14:10, Aditya Srivastava <yashsri421 at gmail.com>
>>>>> wrote:
>>>>>
>>>>>> Currently checkpatch warns us if there is no 'Signed-off-by' line
>>>>>> for the patch.
>>>>>>
>>>>>> E.g., running checkpatch on commit f68e7927212f ("Revert
>>>>>> "powerpc/book3s32: Reorder _PAGE_XXX flags to simplify TLB handling"")
>>>>>> reports this error:
>>>>>>
>>>>>> ERROR: Missing Signed-off-by: line(s)
>>>>>>
>>>>>> Provide a fix by adding a Signed-off-by line corresponding to the author
>>>>>> of the patch before the start of diff(s)
>>>>>>
>>>>>
>>>>> Can you please provide an evaluation first?
>>>>>
>>>>> As far as remember, the fix is often not to add a Signed-off-by: tag but it
>>>>> requires something completely different.
>>>>>
>>>>
>>>> I have generated the 'git logs' of the commits causing
>>>> MISSING_SIGN_OFF at:
>>>> https://github.com/AdityaSrivast/kernel-tasks/blob/master/Task7/summary_log.txt
>>>>
>>>> Actually this fix was suggested by Joe at:
>>>> https://lore.kernel.org/linux-kernel-mentees/e8fef5fbd71331b010988769b249af6a79048f48.camel@perches.com/
>>>>
>>>
>>> Yes I know. Now look at the reports.
>>>
>>> The rc and release commit tags do not get a Signed-off-by
>>> intentionally. Other than that, reverts (for reverts is probably best
>>> to explain how to do a revert... so that a sign-off-by is included)
>>> and typos.
>>>
>>>
>>> Lukas
>>>
>>
>> Yes, I agree. But for the typos, the user must get BAD_SIGN_OFF
>> warning for "Non-standard signature". So user will have to fix that
>> manually. If so, this error is already avoided.
>>
> 
> But then the fix should not be applied if there is a BAD_SIGN_OFF
> warning, right?
> 
> Typos can also be fixed automatically by a fuzzy match. That is a
> different feature, though and a different (next) patch.
> 
Okay.

>> For the release commits, I think the user(s) may not use checkpatch
>>
> 
> Agree. You can filter those out of your evaluation.
> 
>> For the rest (ie reverts) with no sign-offs, I think that if we just
>> add the sign off line, it serves our purpose.
>> Of course if we explain them to revert with sign-off, it will be
>> probably the same outcome.
>>
> 
> Then do both. Provide a fix in checkpatch.pl and provide documentation.
> 
Okay.

> So, how many are because of reverts and how many are because of typos?
> 

Reverts: 4,
Typos: 5
True positives: 2 (3c0edea9b29f9be6, 9ac060a708e05423)

> Also, how many reverts do currently include a proper sign-off?
>> When the majority does not include a proper sign-off on revert commit,
> maybe the convention is to not have a sign-off on reverts.
> When we have some data on that, we can discuss this with some central
> maintainers in the kernel community.
> 
> If that is the case, then the fix is to change checkpatch.pl not to
> complain on revert commits because of a missing sign-off.
>
So, there are 993 commits which are reverts. Out of these only 4 are
there which have missing sign offs.(over 4.13..5.8)

I have a doubt, How can I provide documentation? Couldn't find any doc
file for checkpatch

Thanks
Aditya