Re: [Xen-devel] [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: [Xen-devel] [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"  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 thisif ($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: [Xen-devel] [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 thisif ($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

[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 
---
 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 
+R: Designated reviewer: FullName 
+   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 '
 --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