Add Designated Reviewer (R:) to MAINTAINERS (using a test case)
cover Add Designated Reviewer (R:) to MAINTAINERS (using a test case)
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(-)
Signature · 7 lines
2.13.0 _______________________________________________ Xen-devel mailing list [email protected] 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
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.
--- MAINTAINERS | 2 ++ scripts/get_maintainer.pl | 24 ++++++++++++++++++++++-- 2 files changed, 24 insertions(+), 2 deletions(-)
MAINTAINERS +2 -0
@@ -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.
scripts/get_maintainer.pl +22 -2
@@ -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") {
Signature · 7 lines
2.13.0 _______________________________________________ Xen-devel mailing list [email protected] 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 writes ("[RFC PATCH 1/2] Add Designated Reviewer (R:) to MAINTAINERS file and add support for it in get_maintainer.pl"):
Quoted 2 lines
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).
...
scripts/get_maintainer.pl if (!GetOptions( · line 204
Quoted 3 lines
scripts/get_maintainer.pl +1
-0'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.
scripts/get_maintainer.pl sub add_categories { · line 1114
Quoted 13 lines
scripts/get_maintainer.pl +13
-0+ } 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
[email protected]
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
On 13/04/2018, 18:14, "Ian Jackson" <[email protected]> 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.
scripts/get_maintainer.pl sub add_categories { · line 1114
Quoted 13 lines
scripts/get_maintainer.pl +13
-0+ } 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
[email protected]
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 writes ("Re: [RFC PATCH 1/2] Add Designated Reviewer (R:) to MAINTAINERS file and add support for it in get_maintainer.pl"):
Quoted 1 line
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
[email protected]
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
On 13/04/2018, 18:24, "Ian Jackson" <[email protected]> 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
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
RFC PATCH 2/2 Add Brian Woods as Designated reviewer to AMD IOMMU and AMD SVM
In reply to Lars KurthThis was discussed in an IRC discussion post the April x86 meeting.
--- MAINTAINERS | 2 ++ 1 file changed, 2 insertions(+)
MAINTAINERS +2 -0
@@ -144,12 +144,14 @@ F: tools/libacpi/ AMD IOMMU M: Suravee Suthikulpanit <[email protected]> +R: Brian Woods <[email protected]> S: Maintained F: xen/drivers/passthrough/amd/ AMD SVM M: Boris Ostrovsky <[email protected]> M: Suravee Suthikulpanit <[email protected]> +R: Brian Woods <[email protected]> S: Supported F: xen/arch/x86/hvm/svm/ F: xen/arch/x86/cpu/vpmu_amd.c
Signature · 7 lines
2.13.0 _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel