[V2,3/3] get_maintainer: prevent keywords from matching filenames

Commit Message

Stephen Warren March 7, 2013, 12:29 a.m. UTC

From: Stephen Warren <[e-mail]>

This reverts most of eb90d08 "get_maintainer: allow keywords to match
filenames"; all except the parts that are required to implement the new
N: entry type.

The rationale is that it's better to have K: match just patch or file
content as it previously did, and N: match just filenames, rather than
have K: math all three cases. This gives more explicit control, and
removes the temptation to use K: for filenames, and then have those
keywords accidentally match unexpected patch or file content.

Signed-off-by: Stephen Warren <[e-mail]>
---
v2: New patch.
---
 MAINTAINERS               |    9 ++++-----
 scripts/get_maintainer.pl |    2 +-
 2 files changed, 5 insertions(+), 6 deletions(-)

Comments

Joe Perches March 7, 2013, 12:30 a.m. UTC | #1

On Wed, 2013-03-06 at 17:29 -0700, Stephen Warren wrote:
> From: Stephen Warren <[e-mail]>
> 
> This reverts most of eb90d08 "get_maintainer: allow keywords to match
> filenames"; all except the parts that are required to implement the new
> N: entry type.

Just combine patches 1 and 3 into a single patch.

Stephen Warren March 7, 2013, 12:34 a.m. UTC | #2

On 03/06/2013 05:30 PM, Joe Perches wrote:
> On Wed, 2013-03-06 at 17:29 -0700, Stephen Warren wrote:
>> From: Stephen Warren <[e-mail]>
>>
>> This reverts most of eb90d08 "get_maintainer: allow keywords to match
>> filenames"; all except the parts that are required to implement the new
>> N: entry type.
> 
> Just combine patches 1 and 3 into a single patch.

That would break bisectability; using MAINTAINERS after applying a
squashed 1+3 but without patch 2 applied would not operate as desired.

Joe Perches March 7, 2013, 12:40 a.m. UTC | #3

On Wed, 2013-03-06 at 17:34 -0700, Stephen Warren wrote:
> On 03/06/2013 05:30 PM, Joe Perches wrote:
> > On Wed, 2013-03-06 at 17:29 -0700, Stephen Warren wrote:
> >> From: Stephen Warren <[e-mail]>
> >>
> >> This reverts most of eb90d08 "get_maintainer: allow keywords to match
> >> filenames"; all except the parts that are required to implement the new
> >> N: entry type.
> > 
> > Just combine patches 1 and 3 into a single patch.
> 
> That would break bisectability; using MAINTAINERS after applying a
> squashed 1+3 but without patch 2 applied would not operate as desired.

<smile>

Which is why I showed the whole thing in a single patch.
No worries if it's simply to increase the patch count...

cheers, Joe

Stephen Warren March 7, 2013, 6:47 a.m. UTC | #4

On 03/06/2013 05:40 PM, Joe Perches wrote:
> On Wed, 2013-03-06 at 17:34 -0700, Stephen Warren wrote:
>> On 03/06/2013 05:30 PM, Joe Perches wrote:
>>> On Wed, 2013-03-06 at 17:29 -0700, Stephen Warren wrote:
>>>> From: Stephen Warren <[e-mail]>
>>>>
>>>> This reverts most of eb90d08 "get_maintainer: allow keywords to match
>>>> filenames"; all except the parts that are required to implement the new
>>>> N: entry type.
>>>
>>> Just combine patches 1 and 3 into a single patch.
>>
>> That would break bisectability; using MAINTAINERS after applying a
>> squashed 1+3 but without patch 2 applied would not operate as desired.
> 
> <smile>
> 
> Which is why I showed the whole thing in a single patch.
> No worries if it's simply to increase the patch count...

I'm not sure what showed refers to?

I guess if I squashed /everything/ into a single commit (i.e. the change
to the Tegra section in MAINTAINERS too) then there wouldn't be any
bisect issues. I really don't care about patch count. The reason for >1
patch is that there's often push-back if doing logically separate things
(adding N: feature, modifying a MAINTAINERS entry to use it, removing
part of K: feature) in a single patch...

Joe Perches March 7, 2013, 6:55 a.m. UTC | #5

On Wed, 2013-03-06 at 23:47 -0700, Stephen Warren wrote:
> On 03/06/2013 05:40 PM, Joe Perches wrote:
> > Which is why I showed the whole thing in a single patch.
> > No worries if it's simply to increase the patch count...
> 
> I'm not sure what showed refers to?

The changes I posted with the suggestion
https://lkml.org/lkml/2013/3/6/468

> I guess if I squashed /everything/ into a single commit (i.e. the change
> to the Tegra section in MAINTAINERS too) then there wouldn't be any
> bisect issues. I really don't care about patch count. The reason for >1
> patch is that there's often push-back if doing logically separate things
> (adding N: feature, modifying a MAINTAINERS entry to use it, removing
> part of K: feature) in a single patch...

Not from me anyway.

I think it's a single logical change and
it's trivial too.

No worries whatever happens.

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index 2d02ab5..e68a07a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -100,13 +100,12 @@  Descriptions of section entries:
 	   X:	net/ipv6/
 	   matches all files in and below net excluding net/ipv6/
 	K: Keyword perl extended regex pattern to match content in a
-	   patch or file, or an affected filename.  For instance:
+	   patch or file.  For instance:
 	   K: of_get_profile
-	      matches patch or file content, or filenames, that contain
-	      "of_get_profile"
+	      matches patches or files that contain "of_get_profile"
 	   K: \b(printk|pr_(info|err))\b
-	      matches patch or file content, or filenames, that contain one or
-	      more of the words printk, pr_info or pr_err
+	      matches patches or files that contain one or more of the words
+	      printk, pr_info or pr_err
 	   One regex pattern per line.  Multiple K: lines acceptable.
 
 Note: For the hard of thinking, this list is meant to remain in alphabetical
diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index 27dc5cb..5e4fb14 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -611,7 +611,7 @@  sub get_maintainers {
 				    $hash{$tvi} = $value_pd;
 				}
 			    }
-			} elsif ($type eq 'K' || $type eq 'N') {
+			} elsif ($type eq 'N') {
 			    if ($file =~ m/$value/x) {
 				$hash{$tvi} = 0;
 			    }