[PATCH] get_maintainers.pl: Avoid THE_REST when files are added or removed

[PATCH] get_maintainers.pl: Avoid THE_REST when files are added or removed

[Alan Robinson]

When files are added or removed /dev/null is used as a place
holder name in the patch for the absent file.  Don't try and
find a MAINTAINER for this place holder, it only ever flags
and then spams THE REST.

Signed-off-by: Alan Robinson <Alan.Robinson@ts.fujitsu.com>
---

Tested by pretending to remove xen/common/cpupool.c for
which get_maintainers.pl currently flags 11 addresses, after
this patch only xen-devel and its two maintainers (Juergen and
Dario) get listed...

 scripts/get_maintainer.pl | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index 0ce2d367fa..277ba17a67 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -447,7 +447,9 @@ foreach my $file (@ARGV) {
 		$filename =~ s@^[^/]*/@@;
 		$filename =~ s@\n@@;
 		$lastfile = $filename;
-		push(@files, $filename);
+		if ($filename ne "dev/null") { #Ignore the no-file placeholder
+		     push(@files, $filename);
+		}
 		$patch_prefix = "^[+-].*";	#Now parsing the actual patch
 	    } elsif (m/^\@\@ -(\d+),(\d+)/) {
 		if ($email_git_blame) {
-- 
2.14.1

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] get_maintainers.pl: Avoid THE_REST when files are added or removed

[Jan Beulich]

>>> On 22.02.18 at 10:09, <Alan.Robinson@ts.fujitsu.com> wrote:
> When files are added or removed /dev/null is used as a place
> holder name in the patch for the absent file.  Don't try and
> find a MAINTAINER for this place holder, it only ever flags
> and then spams THE REST.

When a file is added, it falls under THE REST maintainership unless
there's a pattern already in place covering the file, or a new entry
is being added at the same time. When a file is removed, whoever
was its maintainer should be Cc-ed. So perhaps it's rather the
/dev/null placeholder use that's wrong?

Also, please send patches _to_ the list, _cc_-ing individuals as
needed.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] get_maintainers.pl: Avoid THE_REST when files are added or removed

[Alan Robinson]

Hi Jan,

On Thu, Feb 22, 2018 at 02:16:23AM -0700, Jan Beulich wrote:
> From: Jan Beulich <JBeulich@suse.com>
> Subject: Re: [PATCH] get_maintainers.pl: Avoid THE_REST when files are
>  added or removed
> 
> >>> On 22.02.18 at 10:09, <Alan.Robinson@ts.fujitsu.com> wrote:
> > When files are added or removed /dev/null is used as a place
> > holder name in the patch for the absent file.  Don't try and
> > find a MAINTAINER for this place holder, it only ever flags
> > and then spams THE REST.
> 
> When a file is added, it falls under THE REST maintainership unless
> there's a pattern already in place covering the file, or a new entry
> is being added at the same time. When a file is removed, whoever
> was its maintainer should be Cc-ed. So perhaps it's rather the
> /dev/null placeholder use that's wrong?
Exactly - that what this patch tries to do...

get_maintainers.pl collects filename from the '+++' and '---' lines. A 
normal (not add/remove) patch looks like this:

diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index 0ce2d367fa..277ba17a67 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl

An add/delete patch looks like this:

diff --git a/xen/common/cpupool.c b/xen/common/cpupool.c
deleted file mode 100644
index 999839444e..0000000000
--- a/xen/common/cpupool.c
+++ /dev/null

Here get_maintainers.pl finds two files, change _just_ ignores the
dev/null.

> 
> Also, please send patches _to_ the list, _cc_-ing individuals as
> needed.
Ahh..sorry about that

Alan

-- 
Alan Robinson
Enterprise Platform Services, Germany

Fujitsu
Mies-van-der-Rohe-Str. 8, 80807 Muenchen, Deutschland
Tel.: +49 (89) 62060 3927
Mob.: +49 (172) 8512843
E-Mail: Alan.Robinson@ts.fujitsu.com
Web: http://www.fujitsu.com/de/

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] get_maintainers.pl: Avoid THE_REST when files are added or removed

[Jan Beulich]

>>> On 22.02.18 at 10:27, <Alan.Robinson@ts.fujitsu.com> wrote:
> On Thu, Feb 22, 2018 at 02:16:23AM -0700, Jan Beulich wrote:
>> From: Jan Beulich <JBeulich@suse.com>
>> Subject: Re: [PATCH] get_maintainers.pl: Avoid THE_REST when files are
>>  added or removed
>> 
>> >>> On 22.02.18 at 10:09, <Alan.Robinson@ts.fujitsu.com> wrote:
>> > When files are added or removed /dev/null is used as a place
>> > holder name in the patch for the absent file.  Don't try and
>> > find a MAINTAINER for this place holder, it only ever flags
>> > and then spams THE REST.
>> 
>> When a file is added, it falls under THE REST maintainership unless
>> there's a pattern already in place covering the file, or a new entry
>> is being added at the same time. When a file is removed, whoever
>> was its maintainer should be Cc-ed. So perhaps it's rather the
>> /dev/null placeholder use that's wrong?
> Exactly - that what this patch tries to do...
> 
> get_maintainers.pl collects filename from the '+++' and '---' lines. A 
> normal (not add/remove) patch looks like this:
> 
> diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
> index 0ce2d367fa..277ba17a67 100755
> --- a/scripts/get_maintainer.pl
> +++ b/scripts/get_maintainer.pl
> 
> An add/delete patch looks like this:
> 
> diff --git a/xen/common/cpupool.c b/xen/common/cpupool.c
> deleted file mode 100644
> index 999839444e..0000000000
> --- a/xen/common/cpupool.c
> +++ /dev/null
> 
> Here get_maintainers.pl finds two files, change _just_ ignores the
> dev/null.

Oh, I see - it's the file name collection code you change, not the
consuming side, I'm sorry for the noise. However, isn't
omitting the leading / a potential problem then? While it's not
very likely for us to gain ./dev/null, it's also not entirely impossible.
Furthermore, shouldn't you move the setting of $lastfile into
the conditional as well?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] get_maintainers.pl: Avoid THE_REST when files are added or removed

[Alan Robinson]

Hi Jan,

On Thu, Feb 22, 2018 at 02:45:48AM -0700, Jan Beulich wrote:
> Oh, I see - it's the file name collection code you change, not the
> consuming side, I'm sorry for the noise. However, isn't
> omitting the leading / a potential problem then?

The leading / is stripped away several lines before this..

> While it's not
> very likely for us to gain ./dev/null, it's also not entirely impossible.

True and this change does mean that get_maintainer.pl would fail to
recognise such a patch as a patch..(just tried)

> Furthermore, shouldn't you move the setting of $lastfile into
> the conditional as well?

Let me send a second version....

Alan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] get_maintainers.pl: Avoid THE_REST when files are added or removed

[Ian Jackson]

Jan Beulich writes ("Re: [PATCH] get_maintainers.pl: Avoid THE_REST when files are added or removed"):
> On 22.02.18 at 10:27, <Alan.Robinson@ts.fujitsu.com> wrote:
> Oh, I see - it's the file name collection code you change, not the
> consuming side, I'm sorry for the noise. However, isn't
> omitting the leading / a potential problem then? While it's not
> very likely for us to gain ./dev/null, it's also not entirely impossible.

If we gain ./dev/null it is because it was a morning in very early
April and whoever acked it and committed it hadn't had enough coffee.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel