Re: [Qemu-devel] [RFC PATCH 5/5] checkpatch: reduce MAINTAINERS update message frequency

[Thomas Huth]

On 26.01.2017 14:28, Paolo Bonzini wrote:
> 
> 
> On 26/01/2017 14:11, Thomas Huth wrote:
>> This is a port of the following commit from the Linux kernel:
>>
>> commit e0d975b1b439c4fef58fbc306c542c94f48bb849
>> Author: Joe Perches <address@hidden>
>> Date:   Wed Dec 10 15:51:49 2014 -0800
>>
>>     checkpatch: reduce MAINTAINERS update message frequency
>>
>>     When files are being added/moved/deleted and a patch contains an update 
>> to
>>     the MAINTAINERS file, assume it's to update the MAINTAINERS file 
>> correctly
>>     and do not emit the "does MAINTAINERS need updating?" message.
>>
>>     Reported by many people.
>>
>>     Signed-off-by: Joe Perches <address@hidden>
>>     Signed-off-by: Andrew Morton <address@hidden>
>>     Signed-off-by: Linus Torvalds <address@hidden>
>>
>> Signed-off-by: Thomas Huth <address@hidden>
>> ---
>>  scripts/checkpatch.pl | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>> index e1be7b3..555a5b6 100755
>> --- a/scripts/checkpatch.pl
>> +++ b/scripts/checkpatch.pl
>> @@ -1300,6 +1300,12 @@ sub process {
>>                      }
>>              }
>>  
>> +# Check if MAINTAINERS is being updated.  If so, there's probably no need to
>> +# emit the "does MAINTAINERS need updating?" message on file add/move/delete
>> +            if ($line =~ /^\s*MAINTAINERS\s*\|/) {
>> +                    $reported_maintainer_file = 1;
>> +            }
>> +
>>  # Check for added, moved or deleted files
>>              if (!$reported_maintainer_file && !$in_commit_log &&
>>                  ($line =~ /^(?:new|deleted) file mode\s*\d+\s*$/ ||
>>
> 
> Maybe leave it as a warning given this change?

I think chances are high that it still pops up quite frequently with
false positives:

1) The above regex only triggers for patches that contain a diffstat. If
you run the script on patches without diffstat, you always get the
warning as soon as you add, delete or move a file, even if you update
the MAINTAINERS file in the same patch.

2) I think it is quite common in patch series to first introduce new
files in the first patches, and then update MAINTAINERS only once at the
end.

3) The MAINTAINERS file often covers whole folders with wildcard
expressions. So if you add/delete/rename a file within such a folder,
you don't need to update MAINTAINERS thanks to the wildcard.

I guess people might be annoyed if checkpatch.pl throws a warning in
these cases. So a "NOTE: ..." sounds more sane to me. But if you like,
we can also start with a WARNING first and only ease it if people start
to complain?

 Thomas