|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH for-4.11 v3 1/1] Add new add_maintainers.pl script to optimise the workflow when using git format-patch with get_maintainer.pl
On 10/05/2018, 12:38, "George Dunlap" <george.dunlap@xxxxxxxxxx> wrote:
On 05/04/2018 09:36 AM, Lars Kurth wrote:
> The tool covers step 2 of the following workflow
>
> Step 1: git format-patch ... -o <patchdir> ...
> Step 2: ./scripts/add_maintainers.pl -d <patchdir>
> This overwrites *.patch files in <patchdir>
> Step 3: git send-email -to xen-devel@xxxxxxxxxxxxxxxxxxxx
<patchdir>/*.patchxm
>
> I manually tested all options and the most common combinations
> on Mac.
>
> Changes since v1:
> - Added RAB (indicated by Juergen on IRC that this is OK)
> - Remove trailing whitespaces
> - Renamed --prefix to --reroll-count
> - Cleaned up short options -v, ... to be in line with git
> - Added --tags|-t option to add AB, RAB and RB emails to CC list
> - Added --insert|-i mode to allow for people adding CCs to commit message
> instead of the e-mail header (the header is the default)
> - Moved common code into functions
> - Added logic, such that the tool only insert's To: and Cc: statements
> which were not there before, allowing for running the tool multiple
times
> on the same <patchdir>
>
> Changes since v2:
> - Deleted --version and related infrastructure
> - Added subroutine prototypes
> - Removed AT and @lists declaration and used \@ in literals
> - Changed usage message and options based on feedback
> - Improved error handling
> - Removed occurances of index() and replaced with regex
> - Removed non-perl idioms
> - Moved uniq statements to normalize and added info on what normalize does
> - Read L: tags from MAINTAINERS file instead of using heuristic
> - Fixed issues related to metacharacters in getmaintainers()
> - Allow multiple -a | --arg values (because of this renamed --args)
> - Identify tags via regex
> - CC's from tags are only inserted in the mail header, never the body
> - That is unless the new option --tagscc is used
> - Added policy processing which includes reworking insert()
> - Replaced -i|--insert with -p|--inspatch and -c|--inscover now using
policies
> - Added new policies to cover for all user requests
> - Rewrote help message to center around usage of policies
> - Reordered some code (e.g. help string first to make code more easily
readable)
>
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Cc: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> Cc: Julien Grall <julien.grall@xxxxxxx>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> Cc: Tim Deegan <tim@xxxxxxx>
> Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> Signed-off-by: Lars Kurth <lars.kurth@xxxxxxxxxx>
> Release-acked-by: Juergen Gross <jgross@xxxxxxxx>
> ---
> scripts/add_maintainers.pl | 512
+++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 512 insertions(+)
> create mode 100755 scripts/add_maintainers.pl
>
> diff --git a/scripts/add_maintainers.pl b/scripts/add_maintainers.pl
> new file mode 100755
> index 0000000000..11ae60d888
> --- /dev/null
> +++ b/scripts/add_maintainers.pl
> @@ -0,0 +1,512 @@
> +#!/usr/bin/perl -w
> +# (c) 2018, Lars Kurth <lars.kurth@xxxxxxxxxx>
> +#
> +# Add maintainers to patches generated with git format-patch
> +#
> +# Usage: perl scripts/add_maintainers.pl [OPTIONS] -patchdir <patchdir>
> +#
> +# Prerequisites: Execute
> +# git format-patch ... -o <patchdir> ...
> +#
> +# ./scripts/get_maintainer.pl is present in the tree
> +#
> +# Licensed under the terms of the GNU GPL License version 2
> +
> +use strict;
> +
> +use Getopt::Long qw(:config no_auto_abbrev);
> +use File::Basename;
> +use List::MoreUtils qw(uniq);
> +
> +sub getmaintainers ($$$);
> +sub gettagsfrompatch ($$$;$);
> +sub normalize ($$);
> +sub insert ($$$$);
> +sub hastag ($$);
> +
> +# Tool Variables
> +my $tool = $0;
> +my $usage = <<EOT;
> +USAGE: $tool [options] (--patchdir | -d) <patchdir>
> +
> +OPTIONS:
> +--------
> + --reroll-count <n> | -v <n>
> + Choose patch files for specific version. This results into the
> + following filters on <patchdir>
> + 0: default - *.patch
> + >1: v<n>*.patch
> + --inspatch (top|ccbody|cc---|none) | -p (top|ccbody|cc---|none)
> + Insert email addresses into *.patch files according to the POLICY
> + See section POLICY:
> + --inscover (top|ccend|none) | -c (top|ccend|none)
> + Insert email addresses into cover letteraccording to the POLICY
> + See section PROCESSING POLICY:
> + --tags | -t
> + Read email addresses from tags and add to CC list.
> + Note that git send-email does not do this. It will add the senders
> + email adress to the CC list though
> + --tagscc
> + Same as tags, only that in this case CCs extracted from tags
> + are treated like CCs that have come from the *.patch file
Not clear on the difference between these.
> + --arg <argument> | -a <argument> ...
> + Arguments passed on to get_maintainer.pl
> + This option can be used multiple times, e.g. -a <a1> -a <a2> ...
> + --verbose
> + Show more output
> + --help | -h
> + Show this help information
> +
> +PROCESSING POLICY:
Why is this called 'policy'? This seems to be definitions.
> +------------------
> + *.patch files consist of several sections relevant to processing:
> + <top>: This is the email header containing email related information
> + It ends with the Subject: line
> + <body>: This is the body that ends up in the commit message
> + It ends with ---
> + <--->: This section contains the actual patches. CCs added here are
> + processed by git send-email, but are not stored in the commit
> + message. Some people add CCs into this section
<---> is not a normal name (how do you say it? "dash-dash-dash"?), and
worse yet might be confused with an option. `--inspatch cc---` looks
like there was some sort of mistake. "Top" would normally mean, "Top of
the body of the mail".
I think it would be better to call these sections <header>, <commit> and
"commit message", and <comment> and "reviewer comment section",
respectively.
> + <ccend>: It ends with '-- '
> +
> + Note that cover letters do not have the <body> section.
> +
> + The following options specifiy how CCs are insertied into *.patch files
> + top: Insert CCs into the email header
> + Insert CCs from *-by: tags and TOs from mailing lists into the
header
> + (this is the default)
> + ccbody: Insert CCs into body
> + Insert CCs from *-by: tags and TOs from mailing lists into the
header
> + unless specified otherwise (via --tagscc).
> + cc---: Insert CCs just after the --- line
> + Insert CCs from *-by: tags and TOs from mailing lists into the
header
> + unless specified otherwise (via --tagscc).
> + ccend: Insert CCs before the '-- ' line
> + Insert CCs from *-by: tags and TOs from mailing lists into the
header
> + unless specified otherwise (via --tagscc).
> + none: Neither insert TO, CCs from --tags nor other CCs
I don't really get this section.
What about having the functionality be something like this? (Obvious
this would need some code changes as well. Also I guessed what the
significance of the `-- ` is in the cover letter, so correct me if I'm
wrong.)
---
USAGE: $tool [options] (--patchdir | -d) <patchdir>
OPTIONS:
--------
--reroll-count <n> | -v <n>
Choose patch files for specific version. This results into the
following filters on <patchdir>
0: default - *.patch
>1: v<n>*.patch
--patchcc (top|commit|comment|none) | -p (top|commit|comment|none)
Insert CC lines into *.patch files in the specified location.
See LOCATIONS for a definition of the various locations.
The default is `top`.
--covercc (top|end|none) | -c (top|end|none)
Insert CC lines into cover letter in the specified location. See
LOCATIONS for a definition of the various locations.
The default is `top`.
--tags | -t
In addition to the output of get_maintainer.pl, include email
addresses from commit tags (e.g., Reviewed-by, Tested-by, &c) in
the list of CC lines to insert.
--arg <argument> | -a <argument> ...
Arguments passed on to get_maintainer.pl
This option can be used multiple times, e.g. -a <a1> -a <a2> ...
--verbose
Show more output
--help | -h
Show this help information
LOCATIONS
---------
*.patch and cover letters files consist of several sections relevant
to processing:
<header>: This is the email header containing email related information
It ends with the Subject: line
<commit>: This is the email body that ends up in the commit message.
It ends with ---. CC lines added here will be checked
into the git tree on commit. Only applicable to normal
patch files.
<comment>: This is the 'comment for reviewers' section, after the
--- but before the diff actually starts. CCs added here
are processed by git send-email, but are not checked into
the git tree on commit. Only applicable to normal patch
files.
<end>: The part of a cover letter just before `-- ` (which normally
begins a diffstat). Only applicable to cover letters.
---
That looks like a good improvement.
Let me have a go at this with the other comments also
Lars
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |