On Thu, 26 Jan 2017 14:39:35 +0100 Thomas Huth <[email protected]> wrote:
> 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 <[email protected]> > >> 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 <[email protected]> > >> Signed-off-by: Andrew Morton <[email protected]> > >> Signed-off-by: Linus Torvalds <[email protected]> > >> > >> Signed-off-by: Thomas Huth <[email protected]> > >> --- > >> 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? I'd vote for 'NOTE:'. I'm always a bit annoyed if checkpatch triggers for a file already covered by 3) (which seems to be the most common case for patches I've been involved with).
