2015-04-30 12:54:09

by Christoph Hellwig

[permalink] [raw]
Subject: get_maintainers.pl is rude, was Re: [PATCH 05/19] USB: inode.c: move assignment out of if () block

On Thu, Apr 30, 2015 at 02:40:04PM +0200, Greg Kroah-Hartman wrote:
> scripts/get_maintainer.pl is bringing up your name for this file as you
> have modified it in the past:

I've probably modified a large part of files in the kernel in the past.

And I'm annoyed about this stupid behavior in get_maintainers.pl.

Joe, can you please fix your bloody script to not report me for every
goddamnt file I touched once? Authorship of changes to a file is
a completely wrong metric. I can see the point to guess it by
non-maintainer signoffs, but authorship is plain wrong and highly
annoying.

If you're unwilling to fix this please at least add a get_maintainers.ignore
file and add me as the first entry. Thanks!


2015-05-02 11:16:39

by Bernd Petrovitsch

[permalink] [raw]
Subject: Re: get_maintainers.pl is rude, was Re: [PATCH 05/19] USB: inode.c: move assignment out of if () block

On Don, 2015-04-30 at 14:54 +0200, Christoph Hellwig wrote:
> On Thu, Apr 30, 2015 at 02:40:04PM +0200, Greg Kroah-Hartman wrote:
> > scripts/get_maintainer.pl is bringing up your name for this file as you
> > have modified it in the past:
>
> I've probably modified a large part of files in the kernel in the past.
[...]
Does it make sense(Is it enough to filter the changes to only include
changes from the last 6 months or so (if there are newer changes;-)?

People probably don't care if they didn't touch it for ages anyway.

Or ignore changes which touch only a few lines completely (to not
include typo fixes or trivial fixups)?

Kind regards,
Bernd
--
"I dislike type abstraction if it has no real reason. And saving
on typing is not a good reason - if your typing speed is the main
issue when you're coding, you're doing something seriously wrong."
- Linus Torvalds

2015-05-02 13:52:24

by Joe Perches

[permalink] [raw]
Subject: Re: get_maintainers.pl is rude, was Re: [PATCH 05/19] USB: inode.c: move assignment out of if () block

On Thu, 2015-04-30 at 14:54 +0200, Christoph Hellwig wrote:
> Joe, can you please fix your bloody script to not report me for every
> goddamnt file I touched once?

For files that have no specific maintainer, the
default period for reporting commit sign-offs and
authors as possible interested parties in a patch is
1 year.

And given how little my sweet wife likes me when I
swear around her, I'm not inclined to do that
specifically for you, no.

> Authorship of changes to a file is
> a completely wrong metric..

That was Andrew Morton's idea.
https://lkml.org/lkml/2013/10/17/460

> I can see the point to guess it by
> non-maintainer signoffs, but authorship is plain wrong and highly
> annoying.

That's an assertion, but you've neglected to give
a rationale for it. I think authorship is quite a
good reason to be cc'd on something as given that
you've spent the effort to originate code, you're
also quite likely to be interested in patches for
that code.

> If you're unwilling to fix this please at least add a get_maintainers.ignore
> file and add me as the first entry. Thanks!

If a get_maintainers.ignore file is created,
(which seems like a reasonable idea, thanks),
I'm not maintaining it.

Maybe this: (if you create a .get_maintainer.ignore file
with your name in it like)

$ cat .get_maintainer.ignore
Christoph Hellwig <[email protected]>
$
---
scripts/get_maintainer.pl | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)

diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index d701627..fc169fd 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -186,6 +186,27 @@ if (-f $conf) {
unshift(@ARGV, @conf_args) if @conf_args;
}

+my @ignore_emails = ();
+my $ignore_file = which_conf(".get_maintainer.ignore");
+if (-f $ignore_file) {
+ open(my $ignore, '<', "$ignore_file")
+ or warn "$P: Can't find a readable .get_maintainer.ignore file $!\n";
+ while (<$ignore>) {
+ my $line = $_;
+
+ $line =~ s/\s*\n?$//;
+ $line =~ s/^\s*//;
+ $line =~ s/\s+$//;
+ $line =~ s/#.*$//;
+
+ next if ($line =~ m/^\s*$/);
+ if (rfc822_valid($line)) {
+ push(@ignore_emails, $line);
+ }
+ }
+ close($ignore);
+}
+
if (!GetOptions(
'email!' => \$email,
'git!' => \$email_git,
@@ -513,6 +534,16 @@ if ($web) {

exit($exit);

+sub ignore_email_address {
+ my ($address) = @_;
+
+ foreach my $ignore (@ignore_emails) {
+ return 1 if ($ignore eq $address);
+ }
+
+ return 0;
+}
+
sub range_is_maintained {
my ($start, $end) = @_;

@@ -1868,6 +1899,7 @@ sub vcs_assign {
my $percent = $sign_offs * 100 / $divisor;

$percent = 100 if ($percent > 100);
+ next if (ignore_email_address($line));
$count++;
last if ($sign_offs < $email_git_min_signatures ||
$count > $email_git_max_maintainers ||



2015-05-05 16:10:47

by Christoph Hellwig

[permalink] [raw]
Subject: Re: get_maintainers.pl is rude, was Re: [PATCH 05/19] USB: inode.c: move assignment out of if () block

On Sat, May 02, 2015 at 06:52:19AM -0700, Joe Perches wrote:
> > I can see the point to guess it by
> > non-maintainer signoffs, but authorship is plain wrong and highly
> > annoying.
>
> That's an assertion, but you've neglected to give
> a rationale for it. I think authorship is quite a
> good reason to be cc'd on something as given that
> you've spent the effort to originate code, you're
> also quite likely to be interested in patches for
> that code.

It's not. Patch authorship != driver authorship. There are people
like me or Al that keep fixing interface all over the tree. That doesn't
really mean I need patches touching those same files again in my inbox.
I generally couldn't care less, and if I do I will pick it up through
the mailinglists.

> > If you're unwilling to fix this please at least add a get_maintainers.ignore
> > file and add me as the first entry. Thanks!
>
> If a get_maintainers.ignore file is created,
> (which seems like a reasonable idea, thanks),
> I'm not maintaining it.
>
> Maybe this: (if you create a .get_maintainer.ignore file
> with your name in it like)

If we can't get the bullshit heuristics fixed properly please add this
support, and feel free to use me as the initial seed for it.

2015-05-05 16:32:35

by Joe Perches

[permalink] [raw]
Subject: Re: get_maintainers.pl is rude, was Re: [PATCH 05/19] USB: inode.c: move assignment out of if () block

On Tue, 2015-05-05 at 17:38 +0200, Christoph Hellwig wrote:
> On Sat, May 02, 2015 at 06:52:19AM -0700, Joe Perches wrote:
> > > I can see the point to guess it by
> > > non-maintainer signoffs, but authorship is plain wrong and highly
> > > annoying.

The commit signer/author lookup is only done for files
that do not have a specified maintainer.

> > That's an assertion, but you've neglected to give
> > a rationale for it. I think authorship is quite a
> > good reason to be cc'd on something as given that
> > you've spent the effort to originate code, you're
> > also quite likely to be interested in patches for
> > that code.
>
> It's not. Patch authorship != driver authorship. There are people
> like me or Al that keep fixing interface all over the tree. That doesn't
> really mean I need patches touching those same files again in my inbox.
> I generally couldn't care less, and if I do I will pick it up through
> the mailinglists.

There are also people that author patches that are
interested in follow-on changes too.

I also touch many files across the kernel, and I get
the odd random patches I'm not uninterested in. I have
no issue with the 3 seconds it takes to ignore them.

> If we can't get the bullshit heuristics fixed properly please add this
> support, and feel free to use me as the initial seed for it.

As I said, I'm not interested in doing anything to any
.ignore file.

I'll submit the patch to the script, you can submit the
patch to add your name to the .ignore file.

cheers, Joe