[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH 5/5] checkpatch: reduce MAINTAINERS update m
From: |
Cornelia Huck |
Subject: |
Re: [Qemu-devel] [RFC PATCH 5/5] checkpatch: reduce MAINTAINERS update message frequency |
Date: |
Thu, 26 Jan 2017 15:03:39 +0100 |
On Thu, 26 Jan 2017 14:39:35 +0100
Thomas Huth <address@hidden> 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 <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?
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).
- Re: [Qemu-devel] [RFC PATCH 1/5] checkpatch: add a check for utf-8 in commit logs, (continued)
- [Qemu-devel] [RFC PATCH 2/5] checkpatch: check utf-8 content from a commit log when it's missing from charset, Thomas Huth, 2017/01/26
- [Qemu-devel] [RFC PATCH 4/5] checkpatch: emit a reminder about MAINTAINERS on file add/move/delete, Thomas Huth, 2017/01/26
- [Qemu-devel] [RFC PATCH 3/5] checkpatch: ignore email headers better, Thomas Huth, 2017/01/26
- [Qemu-devel] [RFC PATCH 5/5] checkpatch: reduce MAINTAINERS update message frequency, Thomas Huth, 2017/01/26
- Re: [Qemu-devel] [RFC PATCH 5/5] checkpatch: reduce MAINTAINERS update message frequency, Stefan Hajnoczi, 2017/01/30