[Qemu-devel] [PATCH 0/5] MAINTAINERS/get_maintainers: add support for reviewers

[Qemu-devel] [PATCH 0/5] MAINTAINERS/get_maintainers: add support for reviewers

[Paolo Bonzini]

I would like to use "R" so that people can delegate maintenance of some
code while keeping some degree of oversight.  In my case, this can be
appropriate for record/replay and cpus.c.

Other people are free to use "R" as in Linux, where it is used to
designate non-maintainers for reviewing patches in a given area.

Paolo

Brian Norris (2):
  get_maintainer: it's '--pattern-depth', not '-pattern-depth'
  get_maintainer: --r (list reviewer) is on by default

Joe Perches (2):
  get_maintainer: Teach get_maintainer.pl about the new "R:" tag
  get_maintainer: add subsystem to reviewer output

Paul E. McKenney (1):
  MAINTAINERS: Add "R:" tag for self-appointed reviewers

 MAINTAINERS               |  2 ++
 scripts/get_maintainer.pl | 53 +++++++++++++++++++++++++++++++++--------------
 2 files changed, 39 insertions(+), 16 deletions(-)

-- 
2.12.2

[Qemu-devel] [PATCH 1/5] MAINTAINERS: Add "R:" tag for self-appointed reviewers

[Paolo Bonzini]

From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>

Some people are not content with the amount of mail they get, and would
like to be CCed on patches for areas they do not maintain.  Let them
satisfy their own appetite for qemu-devel messages.

Seriously: the purpose here is a bit different from the Linux kernel.
While Linux uses "R" to designate non-maintainers for reviewing patches
in a given area, in QEMU I would also like to use "R" so that people can
delegate sending pull requests while keeping some degree of oversight.

Based on Linux commit eafbaac3093760d1fd3b2a5b9f016362dd68af36.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 MAINTAINERS | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index a1d2b3a4d3..46cf4870f9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12,6 +12,8 @@ consult qemu-devel and not any specific individual privately.
 Descriptions of section entries:
 
 	M: Mail patches to: FullName <address@domain>
+	R: Designated reviewer: FullName <address@domain>
+	   These reviewers should be CCed on patches.
 	L: Mailing list that is relevant to this area
 	W: Web-page with status/info
 	Q: Patchwork web based patch tracking system site
-- 
2.12.2

Re: [Qemu-devel] [PATCH 1/5] MAINTAINERS: Add "R:" tag for self-appointed reviewers

[John Snow]

On 05/03/2017 05:43 AM, Paolo Bonzini wrote:
> From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> 
> Some people are not content with the amount of mail they get, and would
> like to be CCed on patches for areas they do not maintain.  Let them
> satisfy their own appetite for qemu-devel messages.
> 
> Seriously: the purpose here is a bit different from the Linux kernel.
> While Linux uses "R" to designate non-maintainers for reviewing patches
> in a given area, in QEMU I would also like to use "R" so that people can
> delegate sending pull requests while keeping some degree of oversight.
> 

Hm? I don't follow -- you mean to say that "R" is also a designated
pull-request-sender? Did you document that anywhere?

--js

> Based on Linux commit eafbaac3093760d1fd3b2a5b9f016362dd68af36.
> 
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  MAINTAINERS | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a1d2b3a4d3..46cf4870f9 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -12,6 +12,8 @@ consult qemu-devel and not any specific individual privately.
>  Descriptions of section entries:
>  
>  	M: Mail patches to: FullName <address@domain>
> +	R: Designated reviewer: FullName <address@domain>
> +	   These reviewers should be CCed on patches.
>  	L: Mailing list that is relevant to this area
>  	W: Web-page with status/info
>  	Q: Patchwork web based patch tracking system site
> 

Re: [Qemu-devel] [PATCH 1/5] MAINTAINERS: Add "R:" tag for self-appointed reviewers

[Paolo Bonzini]

On 03/05/2017 20:23, John Snow wrote:
>>
>> Seriously: the purpose here is a bit different from the Linux kernel.
>> While Linux uses "R" to designate non-maintainers for reviewing patches
>> in a given area, in QEMU I would also like to use "R" so that people can
>> delegate sending pull requests while keeping some degree of oversight.
>>
> Hm? I don't follow -- you mean to say that "R" is also a designated
> pull-request-sender? Did you document that anywhere?

No, "M" would be the pull request sender, while "R" would be me
delegating while keeping an eye on what's going on.

Paolo

[Qemu-devel] [PATCH 2/5] get_maintainer: Teach get_maintainer.pl about the new "R:" tag

[Paolo Bonzini]

From: Joe Perches <joe@perches.com>

We can now designate reviewers in the MAINTAINERS file with the new
"R:" tag, so this commit teaches get_maintainers.pl to add their
email addresses.

Cherry picked from Linux commit c1c3f2c906e35bcb6e4cdf5b8e077660fead14fe,
with fixes to avoid \C as in QEMU commit ba10f729f1 ("get_maintainer.pl:
\C is deprecated", 2015-09-25).

Signed-off-by: Joe Perches <joe@perches.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/get_maintainer.pl | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index 8261bcb1ad..2bde32f64b 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -21,6 +21,7 @@ my $lk_path = "./";
 my $email = 1;
 my $email_usename = 1;
 my $email_maintainer = 1;
+my $email_reviewer = 1;
 my $email_list = 1;
 my $email_subscriber_list = 0;
 my $email_git = 0;
@@ -180,6 +181,7 @@ if (!GetOptions(
 		'remove-duplicates!' => \$email_remove_duplicates,
 		'mailmap!' => \$email_use_mailmap,
 		'm!' => \$email_maintainer,
+		'r!' => \$email_reviewer,
 		'n!' => \$email_usename,
 		'l!' => \$email_list,
 		's!' => \$email_subscriber_list,
@@ -238,7 +240,8 @@ if ($sections) {
 }
 
 if ($email &&
-    ($email_maintainer + $email_list + $email_subscriber_list +
+    ($email_maintainer + $email_reviewer +
+     $email_list + $email_subscriber_list +
      $email_git + $email_git_blame) == 0) {
     die "$P: Please select at least 1 email option\n";
 }
@@ -718,6 +721,7 @@ MAINTAINER field selection options:
     --hg-since => hg history to use (default: $email_hg_since)
     --interactive => display a menu (mostly useful if used with the --git option)
     --m => include maintainer(s) if any
+    --r => include reviewer(s) if any
     --n => include name 'Full Name <addr\@domain.tld>'
     --l => include list(s) if any
     --s => include subscriber only list(s) if any
@@ -1022,6 +1026,22 @@ sub add_categories {
 		    my $role = get_maintainer_role($i);
 		    push_email_addresses($pvalue, $role);
 		}
+	    } elsif ($ptype eq "R") {
+		my ($name, $address) = parse_email($pvalue);
+		if ($name eq "") {
+		    if ($i > 0) {
+			my $tv = $typevalue[$i - 1];
+			if ($tv =~ m/^(.):\s*(.*)/) {
+			    if ($1 eq "P") {
+				$name = $2;
+				$pvalue = format_email($name, $address, $email_usename);
+			    }
+			}
+		    }
+		}
+		if ($email_reviewer) {
+		    push_email_addresses($pvalue, 'reviewer');
+		}
 	    } elsif ($ptype eq "T") {
 		push(@scm, $pvalue);
 	    } elsif ($ptype eq "W") {
-- 
2.12.2

Re: [Qemu-devel] [PATCH 2/5] get_maintainer: Teach get_maintainer.pl about the new "R:" tag

[Thomas Huth]

On 03.05.2017 11:43, Paolo Bonzini wrote:
> From: Joe Perches <joe@perches.com>
> 
> We can now designate reviewers in the MAINTAINERS file with the new
> "R:" tag, so this commit teaches get_maintainers.pl to add their
> email addresses.
> 
> Cherry picked from Linux commit c1c3f2c906e35bcb6e4cdf5b8e077660fead14fe,
> with fixes to avoid \C as in QEMU commit ba10f729f1 ("get_maintainer.pl:
> \C is deprecated", 2015-09-25).

Upstream linux.git changed it to [A-Z] in commit ce8155f7a ... so maybe
use that in QEMU, too, so we stay closer to the Linux version?

 Thomas

Re: [Qemu-devel] [PATCH 2/5] get_maintainer: Teach get_maintainer.pl about the new "R:" tag

[Paolo Bonzini]

On 03/05/2017 12:45, Thomas Huth wrote:
> On 03.05.2017 11:43, Paolo Bonzini wrote:
>> From: Joe Perches <joe@perches.com>
>>
>> We can now designate reviewers in the MAINTAINERS file with the new
>> "R:" tag, so this commit teaches get_maintainers.pl to add their
>> email addresses.
>>
>> Cherry picked from Linux commit c1c3f2c906e35bcb6e4cdf5b8e077660fead14fe,
>> with fixes to avoid \C as in QEMU commit ba10f729f1 ("get_maintainer.pl:
>> \C is deprecated", 2015-09-25).
> 
> Upstream linux.git changed it to [A-Z] in commit ce8155f7a ... so maybe
> use that in QEMU, too, so we stay closer to the Linux version?

We used "." in ba10f729f1, so I stayed consistent with that one.  But we
could also change those periods to [A-Z] en masse.

Paolo

[Qemu-devel] [PATCH 3/5] get_maintainer: it's '--pattern-depth', not '-pattern-depth'

[Paolo Bonzini]

From: Brian Norris <computersforpeace@gmail.com>

Though it appears that Perl's GetOptions will take either, the latter is
not documented in the options listing.

Cherry picked from Linux commit cc7ff0ef6eca3deeea4a424ca47a67c8450d5424.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/get_maintainer.pl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index 2bde32f64b..449bfbfccb 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -748,7 +748,7 @@ Other options:
   --help => show this help information
 
 Default options:
-  [--email --nogit --git-fallback --m --n --l --multiline -pattern-depth=0
+  [--email --nogit --git-fallback --m --n --l --multiline --pattern-depth=0
    --remove-duplicates --rolestats]
 
 Notes:
-- 
2.12.2

[Qemu-devel] [PATCH 4/5] get_maintainer: --r (list reviewer) is on by default

[Paolo Bonzini]

From: Brian Norris <computersforpeace@gmail.com>

We don't consistenly document the default value next to the option
listing, but we do have a list of defaults here, so let's keep it up to
date.

Cherry picked from Linux commit 4f07510df2e8c47fd65b8ffaaf6c5d334d59d598.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/get_maintainer.pl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index 449bfbfccb..bf210c7fa6 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -748,7 +748,7 @@ Other options:
   --help => show this help information
 
 Default options:
-  [--email --nogit --git-fallback --m --n --l --multiline --pattern-depth=0
+  [--email --nogit --git-fallback --m --r --n --l --multiline --pattern-depth=0
    --remove-duplicates --rolestats]
 
 Notes:
-- 
2.12.2

[Qemu-devel] [PATCH 5/5] get_maintainer: add subsystem to reviewer output

[Paolo Bonzini]

From: Joe Perches <joe@perches.com>

Reviewer output currently does not include the subsystem
that matched.  Add it.

Miscellanea:

o Add a get_subsystem_name routine to centralize this

Cherry picked from Linux commit 2a7cb1dc82fc2a52e747b4c496c13f6575fb1790.

Signed-off-by: Joe Perches <joe@perches.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/get_maintainer.pl | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index bf210c7fa6..63257965ce 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -896,20 +896,29 @@ sub find_ending_index {
     return $index;
 }
 
-sub get_maintainer_role {
+sub get_subsystem_name {
     my ($index) = @_;
 
-    my $i;
     my $start = find_starting_index($index);
-    my $end = find_ending_index($index);
 
-    my $role = "unknown";
     my $subsystem = $typevalue[$start];
     if (length($subsystem) > 20) {
 	$subsystem = substr($subsystem, 0, 17);
 	$subsystem =~ s/\s*$//;
 	$subsystem = $subsystem . "...";
     }
+    return $subsystem;
+}
+
+sub get_maintainer_role {
+    my ($index) = @_;
+
+    my $i;
+    my $start = find_starting_index($index);
+    my $end = find_ending_index($index);
+
+    my $role = "unknown";
+    my $subsystem = get_subsystem_name($index);
 
     for ($i = $start + 1; $i < $end; $i++) {
 	my $tv = $typevalue[$i];
@@ -943,16 +952,7 @@ sub get_maintainer_role {
 sub get_list_role {
     my ($index) = @_;
 
-    my $i;
-    my $start = find_starting_index($index);
-    my $end = find_ending_index($index);
-
-    my $subsystem = $typevalue[$start];
-    if (length($subsystem) > 20) {
-	$subsystem = substr($subsystem, 0, 17);
-	$subsystem =~ s/\s*$//;
-	$subsystem = $subsystem . "...";
-    }
+    my $subsystem = get_subsystem_name($index);
 
     if ($subsystem eq "THE REST") {
 	$subsystem = "";
@@ -1040,7 +1040,8 @@ sub add_categories {
 		    }
 		}
 		if ($email_reviewer) {
-		    push_email_addresses($pvalue, 'reviewer');
+		    my $subsystem = get_subsystem_name($i);
+		    push_email_addresses($pvalue, "reviewer:$subsystem");
 		}
 	    } elsif ($ptype eq "T") {
 		push(@scm, $pvalue);
-- 
2.12.2

Re: [Qemu-devel] [PATCH 0/5] MAINTAINERS/get_maintainers: add support for reviewers

[Thomas Huth]

On 03.05.2017 11:43, Paolo Bonzini wrote:
> I would like to use "R" so that people can delegate maintenance of some
> code while keeping some degree of oversight.  In my case, this can be
> appropriate for record/replay and cpus.c.
> 
> Other people are free to use "R" as in Linux, where it is used to
> designate non-maintainers for reviewing patches in a given area.
> 
> Paolo
> 
> Brian Norris (2):
>   get_maintainer: it's '--pattern-depth', not '-pattern-depth'
>   get_maintainer: --r (list reviewer) is on by default
> 
> Joe Perches (2):
>   get_maintainer: Teach get_maintainer.pl about the new "R:" tag
>   get_maintainer: add subsystem to reviewer output
> 
> Paul E. McKenney (1):
>   MAINTAINERS: Add "R:" tag for self-appointed reviewers
> 
>  MAINTAINERS               |  2 ++
>  scripts/get_maintainer.pl | 53 +++++++++++++++++++++++++++++++++--------------
>  2 files changed, 39 insertions(+), 16 deletions(-)

Patches look fine to me, and I think this is a good idea, so:

Reviewed-by: Thomas Huth <thuth@redhat.com>