[RFC PATCH 0/2] Add Designated Reviewer (R:) to MAINTAINERS (using a test case)

[RFC PATCH 0/2] Add Designated Reviewer (R:) to MAINTAINERS (using a test case)

[Lars Kurth]

This follows up from a conversation after the April x86 community call, in which I had
the following action: Lars to propose fixing CC issue in xen.git:MAINTAINERS copying 
the R section entries from Linux.git:MAINTAINERS (will need changes to 
get_maintainers.pl also)

Lars Kurth (2):
  Add Designated Reviewer (R:) to MAINTAINERS file and add support for
    it in get_maintainer.pl
  Add Brian Woods as Designated reviewer to AMD IOMMU and AMD SVM

 MAINTAINERS               |  4 ++++
 scripts/get_maintainer.pl | 24 ++++++++++++++++++++++--
 2 files changed, 26 insertions(+), 2 deletions(-)

-- 
2.13.0


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

[RFC PATCH 1/2] Add Designated Reviewer (R:) to MAINTAINERS file and add support for it in get_maintainer.pl

[Lars Kurth]

The syntax has been copied from the Linux Maintainers file. I moved the following Linux
get_maintainer.pl patches to Xen, fixing up some merge issues (and a bug).

The get_maintainer.pl changes were based on the following git commits
* https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/scripts/get_maintainer.pl?id=
* c1c3f2c906e35bcb6e4cdf5b8e077660fead14fe
* 4f07510df2e8c47fd65b8ffaaf6c5d334d59d598

I have tested on a number of files using mock entries in MAINTAINERS
using ./scripts/get_maintainer.pl -f ...

I also tested --nor to disable the support and it worked as expected.

Signed-off-by: Lars Kurth <lars.kurth@citrix.com>
---
 MAINTAINERS               |  2 ++
 scripts/get_maintainer.pl | 24 ++++++++++++++++++++++--
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index bbda4b9f43..29c0c4b3a7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -60,6 +60,8 @@ appropriate branch.
 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
 	T: SCM tree type and location.  Type is one of: git, hg, quilt, stgit.
diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index 3fb1ad4b69..85bf518704 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -21,6 +21,7 @@ my $xen_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_penguin_chiefs = 0;
@@ -199,6 +200,7 @@ if (!GetOptions(
 		'mailmap!' => \$email_use_mailmap,
 		'drop_the_rest_supporter!' => \$email_drop_the_rest_supporter_if_supporter_found,
 		'm!' => \$email_maintainer,
+        'r!' => \$email_reviewer,
 		'n!' => \$email_usename,
 		'l!' => \$email_list,
 		's!' => \$email_subscriber_list,
@@ -257,7 +259,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_penguin_chiefs + $email_git_blame) == 0) {
     die "$P: Please select at least 1 email option\n";
 }
@@ -791,6 +794,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
@@ -817,7 +821,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:
@@ -1095,6 +1099,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/^([A-Z]):\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.13.0


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

[RFC PATCH 2/2] Add Brian Woods as Designated reviewer to AMD IOMMU and AMD SVM

[Lars Kurth]

This was discussed in an IRC discussion post the April x86 meeting.

Signed-off-by: Lars Kurth <lars.kurth@citrix.com>
---
 MAINTAINERS | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 29c0c4b3a7..eb2c78ee43 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -144,12 +144,14 @@ F:	tools/libacpi/
 
 AMD IOMMU
 M:	Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
+R:	Brian Woods <brian.woods@amd.com>
 S:	Maintained
 F:	xen/drivers/passthrough/amd/
 
 AMD SVM
 M:	Boris Ostrovsky <boris.ostrovsky@oracle.com>
 M:	Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
+R:	Brian Woods <brian.woods@amd.com>
 S:	Supported
 F:	xen/arch/x86/hvm/svm/
 F:	xen/arch/x86/cpu/vpmu_amd.c
-- 
2.13.0


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

Re: [RFC PATCH 1/2] Add Designated Reviewer (R:) to MAINTAINERS file and add support for it in get_maintainer.pl

[Ian Jackson]

Lars Kurth writes ("[RFC PATCH 1/2] Add Designated Reviewer (R:) to MAINTAINERS file and add support for it in get_maintainer.pl"):
> The syntax has been copied from the Linux Maintainers file. I moved the following Linux
> get_maintainer.pl patches to Xen, fixing up some merge issues (and a bug).
...
>  		'm!' => \$email_maintainer,
> +        'r!' => \$email_reviewer,
>  		'n!' => \$email_usename,

Your text editor has some difficulty with the tabs here, resulting in
misalignment.  Set its tab width to 8 and reindent.

> +        } elsif ($ptype eq "R") {
> +        my ($name, $address) = parse_email($pvalue);
> +        if ($name eq "") {
> +            if ($i > 0) {
> +            my $tv = $typevalue[$i - 1];
> +            if ($tv =~ m/^([A-Z]):\s*(.*)/) {
> +                if ($1 eq "P") {
> +                $name = $2;
> +                $pvalue = format_email($name, $address, $email_usename);
> +                }
> +            }
> +            }
> +        }

What on earth is all this    if ($name eq "")  stuff doing ?  Is it
appropriate in this case ?

If it is actually needed, you should probably not just cut and paste
it.

Thanks,
Ian.

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

Re: [RFC PATCH 1/2] Add Designated Reviewer (R:) to MAINTAINERS file and add support for it in get_maintainer.pl

[Lars Kurth]

On 13/04/2018, 18:14, "Ian Jackson" <ian.jackson@citrix.com> wrote:

    Lars Kurth writes ("[RFC PATCH 1/2] Add Designated Reviewer (R:) to MAINTAINERS file and add support for it in get_maintainer.pl"):
    > The syntax has been copied from the Linux Maintainers file. I moved the following Linux
    > get_maintainer.pl patches to Xen, fixing up some merge issues (and a bug).
    ...
    >  		'm!' => \$email_maintainer,
    > +        'r!' => \$email_reviewer,
    >  		'n!' => \$email_usename,
    
    Your text editor has some difficulty with the tabs here, resulting in
    misalignment.  Set its tab width to 8 and reindent.

I noticed: will fix.
    
    > +        } elsif ($ptype eq "R") {
    > +        my ($name, $address) = parse_email($pvalue);
    > +        if ($name eq "") {
    > +            if ($i > 0) {
    > +            my $tv = $typevalue[$i - 1];
    > +            if ($tv =~ m/^([A-Z]):\s*(.*)/) {
    > +                if ($1 eq "P") {
    > +                $name = $2;
    > +                $pvalue = format_email($name, $address, $email_usename);
    > +                }
    > +            }
    > +            }
    > +        }
    
    What on earth is all this    if ($name eq "")  stuff doing ?  Is it
    appropriate in this case ?
    
    If it is actually needed, you should probably not just cut and paste
    it.
    
I don't know whether it is needed. The same code block of code is in Linux and http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=scripts/get_maintainer.pl;h=3fb1ad4b694d68d5f90ad642420d4b50b934ed2b;hb=HEAD  line 1082 onwards

Lars 

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

Re: [RFC PATCH 1/2] Add Designated Reviewer (R:) to MAINTAINERS file and add support for it in get_maintainer.pl

[Ian Jackson]

Lars Kurth writes ("Re: [RFC PATCH 1/2] Add Designated Reviewer (R:) to MAINTAINERS file and add support for it in get_maintainer.pl"):
> I don't know whether it is needed. The same code block of code is in Linux and http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=scripts/get_maintainer.pl;h=3fb1ad4b694d68d5f90ad642420d4b50b934ed2b;hb=HEAD  line 1082 onwards

Linux's MAINTAINERS says this:

        P: Person (obsolete)

The code seems to be doing some kind of indirection.  I suggest we
drop it.  At the very least, we should not make any more copies of
this!

Ian.

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

Re: [RFC PATCH 1/2] Add Designated Reviewer (R:) to MAINTAINERS file and add support for it in get_maintainer.pl

[Lars Kurth]

On 13/04/2018, 18:24, "Ian Jackson" <ian.jackson@citrix.com> wrote:

    Lars Kurth writes ("Re: [RFC PATCH 1/2] Add Designated Reviewer (R:) to MAINTAINERS file and add support for it in get_maintainer.pl"):
    > I don't know whether it is needed. The same code block of code is in Linux and http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=scripts/get_maintainer.pl;h=3fb1ad4b694d68d5f90ad642420d4b50b934ed2b;hb=HEAD  line 1082 onwards
    
    Linux's MAINTAINERS says this:
    
            P: Person (obsolete)
    
    The code seems to be doing some kind of indirection.  I suggest we
    drop it.  At the very least, we should not make any more copies of
    this!
    
I suppose I can take both instances out, given that we don't support "P", when I re-roll the patch and see whether everything works as expected.

Lars 

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