From: Daniel P. Berrangé <berrange@redhat.com>
When seeing a new/deleted/renamed file we check to see if MAINTAINERS
is updated, but we don't give the user a list of files affected, as
we don't want to repeat the same warning many times over.
Using the new file list hook, we can give a single warning at the
end with a list of filenames included.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
scripts/checkpatch.pl | 37 +++++++++++++++++++------------------
1 file changed, 19 insertions(+), 18 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 4a18daa384..d416a6dcf9 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1442,6 +1442,25 @@ sub process_file_list {
join("\n ", @acpi_nontestexpected) .
"\n\nfound in the same patch\n");
}
+
+ my $sawmaintainers = 0;
+ my @maybemaintainers;
+ foreach my $fileinfo (@fileinfos) {
+ if ($fileinfo->{action} ne "modified" &&
+ $fileinfo->{filenew} !~ m#^tests/data/acpi/#) {
+ push @maybemaintainers, $fileinfo->{filenew};
+ }
+ if ($fileinfo->{filenew} eq "MAINTAINEfRS") {
+ $sawmaintainers = 1;
+ }
+ }
+
+ # If we don't see a MAINTAINERS update, prod the user to check
+ if (int(@maybemaintainers) > 0 && !$sawmaintainers) {
+ WARN("added, moved or deleted file(s):\n\n " .
+ join("\n ", @maybemaintainers) .
+ "\n\nDoes MAINTAINERS need updating?\n");
+ }
}
# Called at the start of processing a diff hunk for a file
@@ -1485,7 +1504,6 @@ sub process {
my $in_header_lines = $file ? 0 : 1;
my $in_commit_log = 0; #Scanning lines before patch
- my $reported_maintainer_file = 0;
my $reported_mixing_imported_file = 0;
my $in_imported_file = 0;
my $in_no_imported_file = 0;
@@ -1760,23 +1778,6 @@ 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*$/ ||
- $line =~ /^rename (?:from|to) [\w\/\.\-]+\s*$/ ||
- ($line =~ /\{\s*([\w\/\.\-]*)\s*\=\>\s*([\w\/\.\-]*)\s*\}/ &&
- (defined($1) || defined($2)))) &&
- $realfile !~ m#^tests/data/acpi/#) {
- $reported_maintainer_file = 1;
- WARN("added, moved or deleted file(s), does MAINTAINERS need updating?\n" . $herecurr);
- }
-
# Check SPDX-License-Identifier references a permitted license
if ($rawline =~ m,SPDX-License-Identifier: (.*?)(\*/)?\s*$,) {
&checkspdx($realfile, $1);
--
2.49.0
On 5/12/25 20:24, Daniel P. Berrangé wrote:
> From: Daniel P. Berrangé <berrange@redhat.com>
>
> When seeing a new/deleted/renamed file we check to see if MAINTAINERS
> is updated, but we don't give the user a list of files affected, as
> we don't want to repeat the same warning many times over.
>
> Using the new file list hook, we can give a single warning at the
> end with a list of filenames included.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> scripts/checkpatch.pl | 37 +++++++++++++++++++------------------
> 1 file changed, 19 insertions(+), 18 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 4a18daa384..d416a6dcf9 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1442,6 +1442,25 @@ sub process_file_list {
> join("\n ", @acpi_nontestexpected) .
> "\n\nfound in the same patch\n");
> }
> +
> + my $sawmaintainers = 0;
> + my @maybemaintainers;
> + foreach my $fileinfo (@fileinfos) {
> + if ($fileinfo->{action} ne "modified" &&
> + $fileinfo->{filenew} !~ m#^tests/data/acpi/#) {
> + push @maybemaintainers, $fileinfo->{filenew};
> + }
> + if ($fileinfo->{filenew} eq "MAINTAINEfRS") {
MAINTAINEfRS ? looks like a typo.
With that fixed,
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Thanks,
C.
> + $sawmaintainers = 1;
> + }
> + }
> +
> + # If we don't see a MAINTAINERS update, prod the user to check
> + if (int(@maybemaintainers) > 0 && !$sawmaintainers) {
> + WARN("added, moved or deleted file(s):\n\n " .
> + join("\n ", @maybemaintainers) .
> + "\n\nDoes MAINTAINERS need updating?\n");
> + }
> }
>
> # Called at the start of processing a diff hunk for a file
> @@ -1485,7 +1504,6 @@ sub process {
>
> my $in_header_lines = $file ? 0 : 1;
> my $in_commit_log = 0; #Scanning lines before patch
> - my $reported_maintainer_file = 0;
> my $reported_mixing_imported_file = 0;
> my $in_imported_file = 0;
> my $in_no_imported_file = 0;
> @@ -1760,23 +1778,6 @@ 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*$/ ||
> - $line =~ /^rename (?:from|to) [\w\/\.\-]+\s*$/ ||
> - ($line =~ /\{\s*([\w\/\.\-]*)\s*\=\>\s*([\w\/\.\-]*)\s*\}/ &&
> - (defined($1) || defined($2)))) &&
> - $realfile !~ m#^tests/data/acpi/#) {
> - $reported_maintainer_file = 1;
> - WARN("added, moved or deleted file(s), does MAINTAINERS need updating?\n" . $herecurr);
> - }
> -
> # Check SPDX-License-Identifier references a permitted license
> if ($rawline =~ m,SPDX-License-Identifier: (.*?)(\*/)?\s*$,) {
> &checkspdx($realfile, $1);
On Wed, May 14, 2025 at 01:51:52PM +0200, Cédric Le Goater wrote:
> On 5/12/25 20:24, Daniel P. Berrangé wrote:
> > From: Daniel P. Berrangé <berrange@redhat.com>
> >
> > When seeing a new/deleted/renamed file we check to see if MAINTAINERS
> > is updated, but we don't give the user a list of files affected, as
> > we don't want to repeat the same warning many times over.
> >
> > Using the new file list hook, we can give a single warning at the
> > end with a list of filenames included.
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> > scripts/checkpatch.pl | 37 +++++++++++++++++++------------------
> > 1 file changed, 19 insertions(+), 18 deletions(-)
> >
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > index 4a18daa384..d416a6dcf9 100755
> > --- a/scripts/checkpatch.pl
> > +++ b/scripts/checkpatch.pl
> > @@ -1442,6 +1442,25 @@ sub process_file_list {
> > join("\n ", @acpi_nontestexpected) .
> > "\n\nfound in the same patch\n");
> > }
> > +
> > + my $sawmaintainers = 0;
> > + my @maybemaintainers;
> > + foreach my $fileinfo (@fileinfos) {
> > + if ($fileinfo->{action} ne "modified" &&
> > + $fileinfo->{filenew} !~ m#^tests/data/acpi/#) {
> > + push @maybemaintainers, $fileinfo->{filenew};
> > + }
> > + if ($fileinfo->{filenew} eq "MAINTAINEfRS") {
>
> MAINTAINEfRS ? looks like a typo.
Opps, an intentional typo when I was testing logic that
I accidentally committed.
>
>
> With that fixed,
>
>
> Reviewed-by: Cédric Le Goater <clg@redhat.com>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
© 2016 - 2025 Red Hat, Inc.