[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 4/5] checkpatch: bump most warnings to errors
From: |
Thomas Huth |
Subject: |
Re: [Qemu-devel] [PATCH 4/5] checkpatch: bump most warnings to errors |
Date: |
Wed, 10 Aug 2016 12:48:27 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2 |
On 10.08.2016 12:44, Paolo Bonzini wrote:
>>>>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>>>>> index 714a000..ab08ca2 100755
>>>>> --- a/scripts/checkpatch.pl
>>>>> +++ b/scripts/checkpatch.pl
>>>>> @@ -1289,11 +1289,11 @@ sub process {
>>>>> # This is a signoff, if ugly, so do not double report.
>>>>> $signoff++;
>>>>> if (!($line =~ /^\s*Signed-off-by:/)) {
>>>>> - WARN("Signed-off-by: is the preferred form\n" .
>>>>> + ERROR("Signed-off-by: is the preferred form\n" .
>>>>> $herecurr);
>>>>> }
>>>>
>>>> If you turn this into an ERROR, it's not the "preferred form" anymore,
>>>> but the "mandated form". So I'd suggest to either keep it as WARN or to
>>>> rephrase the message.
>>>
>>> What about:
>>>
>>> ERROR("Signed-off-by: is spelled with
>>> uppercase \"s\"\n" .
>>> $herecurr);
>>
>> That would still be confusing if I'd spell it like "Signed-Off-BY", for
>> example.
>
>
> Right, so I guess "The correct form is \"Signed-off-by\"\n" is more precise.
Yes, that sounds better.
>> Maybe the outer check should simply not be case-insensitive, then you
>> could remove this check here completely?
>
> The reason for that is to hide the "Missing Signed-off-by: line(s)" error.
> See here:
>
> # This is a signoff, if ugly, so do not double report.
> $signoff++;
> ...
> if ($is_patch && $chk_signoff && $signoff == 0) {
> ERROR("Missing Signed-off-by: line(s)\n");
> }
Sure, but I think that error would be OK, too, since most people should
be able to figure out that they spelled "signed-off-by" in a bad way
when they get a "Missing Signed-off-by: line" error.
Thomas
- Re: [Qemu-devel] [PATCH 1/5] checkpatch: tweak the files in which TABs are checked, (continued)
Re: [Qemu-devel] [PATCH v2 0/5] checkpatch tweaks, Markus Armbruster, 2016/08/10
Re: [Qemu-devel] [PATCH v2 0/5] checkpatch tweaks, no-reply, 2016/08/10
Re: [Qemu-devel] [PATCH v2 0/5] checkpatch tweaks, no-reply, 2016/08/10