Add new add_maintainers.pl script to optimise the workflow when using git format-patch with get_maintainer.pl
v3 cover Add new add_maintainers.pl script to optimise the workflow when using git format-patch with get_maintainer.pl
This version of this series addresses all comments on the mailing list, as well as some feedback I got in various personal conversations and/or on IRC. For the people who asked for specific features/workflows:
Ian Jackson: use ./scripts/add_maintainers.pl -p none [-c top] Reads CCs from unmodified *.patch files and inserts them into the cover letter
George Dunlap: use ./scripts/add_maintainers.pl -p cc---
Tends to add CC blocks after the --- line in *.patches. This option achieves this behavior/
Julien Grall: use ./scripts/add_maintainers.pl -c ccend
As far as I recall, Julien adds CC blocks into the body of the cover letter. This option achieves this, but there is no place that always exists other than before "-- " where the CC block can be insterted.
I made the processing code easily extendable via policies. So if there is any missed behavior, the tool can easily be extended.
Also note that git send-email does not automatically add people in *=by:
tags to CC lists (with the exception of Singed-off-by). For this I added the options -t|--tags and --tagscc.
v2 of this patch contained some cleanup to MAINTAINERS which has been broken out into a separate series: see
https://lists.xenproject.org/archives/html/xen-devel/2018-05/threads.html#00028
Lars Kurth (1):
Add new add_maintainers.pl script to optimise the workflow when using
git format-patch with get_maintainer.pl
scripts/add_maintainers.pl | 512 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 512 insertions(+) create mode 100755 scripts/add_maintainers.pl
Signature · 7 lines
2.13.0 _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
PATCH v3 1/1 Add new add_maintainers.pl script to optimise the workflow when using git format-patch with get_maintainer.pl
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 [email protected] <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)
--- scripts/add_maintainers.pl | 512 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 512 insertions(+) create mode 100755 scripts/add_maintainers.pl
scripts/add_maintainers.pl new file +512 -0
@@ -0,0 +1,512 @@ +#!/usr/bin/perl -w +# (c) 2018, Lars Kurth <[email protected]> +# +# 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 + --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: +------------------ + *.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 + <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 + +WORKFLOW: +--------- + This script is intended to be used as part 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> but makes a backup + Step 3: git send-email -to xen-devel\@lists.xenproject.org <patchdir>/*.patch +EOT + +# Constants and functions related to policies + +# Constants for -p|--inspatch and -c|--inscover option processing +my @ppolicies = ("top", "ccbody", "cc---", "none"); +my @cpolicies = ("top", "ccend", "none"); + +# Hash is used to determine which mode value maps onto which search string +my %inssearch = ( + "top" => "Date:", # Insert before Date: + "ccbody" => "Signed-off-by:", # Insert before Signed-off-by: + "cc---" => "---", # Insert after --- + "ccend" => "-- ", # Insert before '-- ' +); + +# Hash is used to determine whether for a given mode we insert CCs after +# the search string or before +my %insafter = ( + "top" => 0, + "ccbody" => 0, + "cc---" => 1, + "ccend" => 0, +); + +# The following subroutines take a areference to arrays of +# - @top: contains CCs from *-by: tags and TOs from mailing lists +# - @cc: contains all other CC's +# It will then apply the corect policies on the input file + +sub applypolicy_top ($$$) { + my ($file, $rtop, $rcc) = @_; + my $insert = join("\n", uniq (@$rtop, @$rcc)); + insert($file , $insert, $inssearch{top}, $insafter{top}); +} + +sub applymixedpolicy ($$$$) { + my ($file, $rtop, $rcc, $mode) = @_; + my $top = join("\n", @$rtop); + my $cc = join("\n", @$rcc); + # Insert snippets into files + insert($file , $cc, $inssearch{$mode}, $insafter{$mode}); + # The top + insert($file , $top, $inssearch{top}, $insafter{top}); +} + +sub applypolicy_ccbody($$$) { + my ($file, $rtop, $rcc) = @_; + applymixedpolicy($file, $rtop, $rcc, "ccbody"); +} + +# Use a different name to make sure perl doesn't throw a syntax error +sub applypolicy_cc3minus ($$$) { + my ($file, $rtop, $rcc) = @_; + applymixedpolicy($file, $rtop, $rcc, "cc---"); +} + +sub applypolicy_ccend ($$$) { + my ($file, $rtop, $rcc) = @_; + applymixedpolicy($file, $rtop, $rcc, "ccend"); +} + +sub applypolicy_none ($$$) { + return; +} + +# Hash for policy functions +my %applypolicy = ( + "top" => \&applypolicy_top, + "ccbody" => \&applypolicy_ccbody, + "cc---" => \&applypolicy_cc3minus, + "ccend" => \&applypolicy_ccend, + "none" => \&applypolicy_none, +); + +# Arguments / Options +my $help = 0; +my $patch_dir = 0; +my @get_maintainer_args = (); +my $verbose = 0; +my $rerollcount = 0; +my $patch_prefix = "0"; # Use a 0, such that v* does not get picked up + # Obviously this will only work for series with + # < 999 patches, which should be fine +my $tags = 0; +my $tagscc = 0; +my $ppolicy = "top"; +my $cpolicy = "top"; + +# Constants +# Keep these as constants, in case we want to make these configurable +# in future +my $CC = "Cc:"; # Note: git-send-mail requires Cc: +my $TO = "To:"; +my $cover_letter = "0000-cover-letter.patch"; +my $get_maintainer = "./scripts/get_maintainer.pl"; +my $patch_ext = ".patch"; +my $maintainers = "MAINTAINERS"; + +if (!GetOptions( + 'd|patchdir=s' => \$patch_dir, + 'v|reroll-count=i' => \$rerollcount, + 'p|inspatch=s' => \$ppolicy, + 'c|inscover=s' => \$cpolicy, + 't|tags' => \$tags, + 'tagscc' => \$tagscc, + 'a|arg=s' => \@get_maintainer_args, + 'verbose' => \$verbose, + 'h|help' => \$help, + )) { + die "$tool: invalid argument - use --help if necessary\n"; +} + +if ($help) { + print $usage; + exit 0; +} + +if (!$patch_dir) { + die "$tool: Directory -d|--patchdir not specified\n"; +} + +if (! -e $patch_dir) { + die "$tool: Directory $patch_dir does not exist\n"; +} + +if ($rerollcount > 0) { + $patch_prefix = "v".$rerollcount; +} + +if ( ! grep $_ eq $ppolicy, @ppolicies ) { + die "$tool: Invalid -p|--inspatch value\n"; +} +if ( ! grep $_ eq $cpolicy, @cpolicies ) { + die "$tool: Invalid -c|--inscover value\n"; +} + +# Get the list of patches +my $has_cover_letter = 0; +my $cover_letter_file; +my $pattern = $patch_dir.'/'.$patch_prefix.'*'.$patch_ext; + +$!=0; +my @patches = glob($pattern); +if ($!) { + die "$tool: Directory $patch_dir contains no patches\n"; +} +if (!scalar @patches) { + die "$tool: Directory $patch_dir contains no matching patches\n"; +} + +# Do the actual processing +my $file; +my @combined_top; +my @combined_cc; + +foreach my $file (@patches) { + if ($file =~ /\Q$cover_letter\E/) { + $has_cover_letter = 1; + $cover_letter_file = $file; + } else { + my @top; # To: lists returned by get_maintainers.pl + my @toppatch; # To: entries in *.patch + # + # Also includes CC's from tags as we do not want + # entries in the body such as + # CC: [email protected] + # ... + # Tested-by: [email protected] + + my @cc; # Cc: maintainers returned by get_maintainers.pl + my @ccpatch; # Cc: entries in *.patch + my @extrapatch; # Cc: for AB, RB, RAB in *.patch + + print "Processing: ".basename($file)."\n"; + + # Read tags from output of get_maintainers.pl + # Lists go into @top and everything else into @cc + getmaintainers($file, \@top, \@cc); + + # Read all lines with CC & TO from the patch file (these will + # likely come from the commit message). Also read tags. + gettagsfrompatch($file, \@toppatch, \@ccpatch, \@extrapatch); + + # With -t|--tags only add @extrapatch to @top and @combined_top + # With --tagscc treat tags as CC that came from the *.patch file + if ($tags & !$tagscc) { + # Copy these always onto the TO related arrays + push @top, @extrapatch; + push @combined_top, @extrapatch; + } elsif ($tagscc) { + # Treat these as if they came from CC's + push @ccpatch, @extrapatch; + push @combined_cc, @extrapatch; + } + + # In this section we normalize the lists. We remove entries + # that are already in the patch, from @cc and @to + my @top_only = normalize(\@top, \@toppatch); + my @cc_only = normalize(\@cc, \@ccpatch); + + # Apply the policy + $applypolicy{$ppolicy}($file, \@top_only, \@cc_only); + } +} + +# Deal with the cover letter +if ($has_cover_letter) { + my @toppatch; # Entries inserted at the top + my @ccpatch; # Cc: entries in *.patch + + print "Processing: ".basename($cover_letter_file)."\n"; + + # Read all lines with CC & TO from the patch file such that subsequent + # calls don't lead to duplication + gettagsfrompatch($cover_letter_file, \@toppatch, \@ccpatch); + + # In this section we normalize the lists. We remove entries + # that are already in the patch, from @cc and @to + my @top_only = normalize(\@combined_top, \@toppatch); + my @cc_only = normalize(\@combined_cc, \@ccpatch); + + # Apply the policy + $applypolicy{$cpolicy}($cover_letter_file, \@top_only, \@cc_only); + + print "\nDon't forget to add the subject and message to ". + $cover_letter_file."\n"; +} + +print "Then perform:\n". + "git send-email -to xen-devel\@lists.xenproject.org ". + $patch_dir.'/'.$patch_prefix."*.patch"."\n"; + +exit 0; + +my $readmailinglists = 0; +my @mailinglists = (); + +sub getmailinglists () { + # Read mailing list from MAINTAINERS file and copy + # a list of e-mail addresses to @mailinglists + if (!$readmailinglists) { + if (-e $maintainers) { + my $fh; + my $line; + open($fh, "<", $maintainers) or die $!; + while (my $line = <$fh>) { + chomp $line; + if ($line =~ /^L:[[:blank:]]+/m) { + push @mailinglists, $'; + } + } + close $fh or die $!; + } else { + print "Warning: file '$maintainers' does not exist\n"; + print "Warning: Mailing lists will be treated as CC's\n"; + } + # Don't try again, even if the MAINTAINERS file does not exist + $readmailinglists = 1; + # Remove any duplicates + @mailinglists = uniq @mailinglists; + } +} + +sub ismailinglist ($) { + my ($check) = @_; + # Get the mailing list information + getmailinglists(); + # Do the check + if ( grep { $_ eq $check} @mailinglists) { + return 1; + } + return 0; +} + +sub getmaintainers ($$$) { + my ($file, $rto, $rcc) = @_; + my $get_maintainer_args = join " ", @get_maintainer_args; + my $cmd = "$get_maintainer $get_maintainer_args <$file"; + my $fh; + + if (! -e $get_maintainer) { + die "$tool: The tool requires $get_maintainer\n"; + } + + open($fh, "-|", $cmd) + or die "Failed to open '$cmd'\n"; + while(my $line = <$fh>) { + chomp $line; + # Keep lists and CC's separately as we dont want them in + # the commit message under a Cc: line + if (ismailinglist($line)) { + push @$rto, $TO." ".$line; + push @combined_top, $TO." ".$line; + } else { + push @$rcc, $CC." ".$line; + push @combined_cc, $CC." ".$line; + } + } + close $fh; +} + +sub gettagsfrompatch ($$$;$) { + my ($file, $rto, $rcc, $rextra) = @_; + my $fh; + + open($fh, "<", $file) + or die "Failed to open '$file'\n"; + while(<$fh>) { + chomp; + my $line = $_; + my $nline; + + if (hastag($line, $TO)) { + push @$rto, $line; + push @combined_top, $line; + } + if (hastag($line, $CC)) { + push @$rcc, $line; + push @combined_cc, $line; + } + # If there is an $rextra, then get various tags and add + # email addresses to the CC list + if ($rextra && $line =~ /^[-0-9a-z]+-by:[[:blank:]]+/mi) { + push @$rextra, $CC." ".$'; + } + } + close $fh; +} + +sub hastag ($$) { + my ($line, $tag) = @_; + if ($line =~ m{^\Q$tag\E}i) { + return 1; + } + return 0; +} + +sub normalize ($$) { + # This function is used to normalize lists of tags or CC / TO lists + # - It removes duplicates in the input arrays + # - It ensures that elements in the second list are not in the first + my ($ra, $rb) = @_; + my @aonly = (); + my %seen; + my $item; + + @$ra = uniq @$ra; + @$rb = uniq @$rb; + foreach $item (@$rb) { + $seen{$item} = 1; + } + foreach $item (@$ra) { + unless ($seen{$item}) { + # it's not in %seen, so add to @aonly + push @aonly, $item; + } + } + + return @aonly; +} + +sub readfile ($) { + my ($file) = @_; + my $fh; + my $content; + open($fh, "<", $file) + or die "Could not open file '$file' $!"; + $content = do { local $/; <$fh> }; + close $fh or die $!; + + return $content; +} + +sub writefile ($$) { + my ($content, $file) = @_; + my $fh; + open($fh, ">", $file) + or die "Could not open file '$file' $!"; + print $fh $content or die $!; + close $fh or die $!; +} + +sub insert ($$$$) { + my ($file, $insert, $delimiter, $insafter) = @_; + my $content; + + if ($insert eq "") { + # Nothing to insert + return; + } + # Read file + $content = readfile($file); + + # Split the string and generate new content + if ($content =~ /^\Q$delimiter\E/mi) { + if ($insafter) { + writefile($`.$delimiter."\n".$insert."\n".$', $file); + + if ($verbose) { + print "\nInserted into ".basename($file).' after "'. + $delimiter."'"."\n-----\n".$insert."\n-----\n"; + } + } else { + writefile($`.$insert."\n".$delimiter.$', $file); + + if ($verbose) { + print "\nInserted into ".basename($file).' before "'. + $delimiter."'"."\n-----\n".$insert."\n-----\n"; + } + } + + } else { + print "Error: Didn't find '$delimiter' in '$file'\n"; + } +}
Signature · 7 lines
2.13.0 _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
Thanks. This is much better :-). I have reviewed this for style, obvious bugs, and the semantics in the doc comment. I haven't tried to follow the algorithm in detail, but I reckon it's probably OK.
I have reordered the patch (and hence the file) to make the grouping of my comments make more sense.
Lars Kurth writes ("[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"):
scripts/add_maintainers.pl line 44
Quoted 6 lines
scripts/add_maintainers.pl new file +6
-0+ --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:
I'm afraid that I don't understand which addresses are added where, from the documentation. In particular, what happens without --tags or --tagscc ? Also you should define `tag'; it has a lot of different meanings, some subtly different, and it is not completely clear which one you mean.
I think you should formally state the default behaviour. Something like:
By default:
* get_maintainer is called on each patch to find email addresses of maintainers/reviewers for that patch; these are added to the patch body near the CC section.
* further email addresses are found in each patch's commit message tags (CC, signed-off-by, reviewed-by, etc.)
* All of the above addresses are added to the CC mail headers of each patch
* All of the above addresses are added to the CC mail headers of the cover letter
I suspect that what I have above is not the real behaviour. You should write what is true in that kind of style :-).
scripts/add_maintainers.pl line 180
Quoted 3 lines
scripts/add_maintainers.pl new file +3
-0+my $patch_prefix = "0"; # Use a 0, such that v* does not get picked up + # Obviously this will only work for series with + # < 999 patches, which should be fine
I don't understand the purpose of this:
Quoted 3 lines
scripts/add_maintainers.pl new file +3
-0+if ($rerollcount > 0) { + $patch_prefix = "v".$rerollcount; +}
...
scripts/add_maintainers.pl line 328
Quoted 4 lines
scripts/add_maintainers.pl new file +4
-0+my $pattern = $patch_dir.'/'.$patch_prefix.'*'.$patch_ext; +print "Then perform:\n". + "git send-email -to xen-devel\@lists.xenproject.org ". + $patch_dir.'/'.$patch_prefix."*.patch"."\n";
What files matching *.patch exist here that should not be processed ? If the answer is none then $patch_prefix is redundant, I think ?
scripts/add_maintainers.pl line 254
Quoted 2 lines
scripts/add_maintainers.pl new file +2
-0+foreach my $file (@patches) { + if ($file =~ /\Q$cover_letter\E/) {
I know you had me look at this over your shoulder and I said it was right, but I think in fact this would match hypothetical files
$patch_dir/0020-do-something-about-0000-cover-letter.patch
I think you need to expect a /. So one of
+ if ($file =~ /\/\Q$cover_letter\E/) {
+ if ($file =~ m{/\Q$cover_letter\E}) {
scripts/add_maintainers.pl line 333
Quoted 8 lines
scripts/add_maintainers.pl new file +8
-0+print "Then perform:\n". + "git send-email -to xen-devel\@lists.xenproject.org ". + $patch_dir.'/'.$patch_prefix."*.patch"."\n"; + +exit 0; + +my $readmailinglists = 0; +my @mailinglists = ();
This is a very curious structure. These assignments are never executed (but I guess the program works anyway). I would recommend moving the main program to the bottom of the file.
scripts/add_maintainers.pl line 338
Quoted 4 lines
scripts/add_maintainers.pl new file +4
-0+sub getmailinglists () { + # Read mailing list from MAINTAINERS file and copy + # a list of e-mail addresses to @mailinglists + if (!$readmailinglists) {
I suggest you rename this variable $getmailinglists_done or something. As it is it is confusing because `read' might be the present tense, but then the sense is wrong.
Also, you might find it better to use a structure like one of
if ($getmailingslists_done) { return; }
return if $getmailingslists_done;Quoted 1 line
+ if (-e $maintainers) {
...
scripts/add_maintainers.pl line 358
Quoted 7 lines
scripts/add_maintainers.pl new file +7
-0+ print "Warning: Mailing lists will be treated as CC's\n"; + } + # Don't try again, even if the MAINTAINERS file does not exist + $readmailinglists = 1; + # Remove any duplicates + @mailinglists = uniq @mailinglists; + }
Indentation here is misleading. (But this will go away if you adopt my suggestion above).
scripts/add_maintainers.pl line 366
Quoted 6 lines
scripts/add_maintainers.pl new file +6
-0+sub ismailinglist ($) { + my ($check) = @_; + # Get the mailing list information + getmailinglists(); + # Do the check + if ( grep { $_ eq $check} @mailinglists) {
Rather than uniq above, and then grep here, you could use a hash %mailinglists. That would be more idiomatic and also less code and faster. But as it is is tolerable.
Quoted 4 lines
scripts/add_maintainers.pl new file +4
-0+sub getmaintainers ($$$) { + my ($file, $rto, $rcc) = @_; + my $get_maintainer_args = join " ", @get_maintainer_args; + my $cmd = "$get_maintainer $get_maintainer_args <$file";
...
scripts/add_maintainers.pl line 383
Quoted 2 lines
scripts/add_maintainers.pl new file +2
-0+ open($fh, "-|", $cmd) + or die "Failed to open '$cmd'\n";
You should use the array form of piped open, rather than this string joining. That way arguments containing spaces make their way through correct.
scripts/add_maintainers.pl line 379
Quoted 2 lines
scripts/add_maintainers.pl new file +2
-0+ if (! -e $get_maintainer) { + die "$tool: The tool requires $get_maintainer\n";
I still don't like this check. What if the user specifies an implementation of $get_maintainer which is on the PATH ?
Quoted 1 line
+ while(my $line = <$fh>) {
...
Review
Quoted 2 lines
(quoted diff) +2
-0+ } + close $fh;
You need to check the errors here. See the `perldoc -f close'.
Quoted 1 line
+ if ($tags & !$tagscc) {
You mean &&, not &.
Thanks,
Ian.
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
On 04/05/2018, 18:43, "Ian Jackson" <[email protected]> wrote:
Thanks. This is much better :-). I have reviewed this for style, obvious bugs, and the semantics in the doc comment. I haven't tried to follow the algorithm in detail, but I reckon it's probably OK.
I have reordered the patch (and hence the file) to make the grouping of my comments make more sense.
Lars Kurth writes ("[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"):
> + --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:
I'm afraid that I don't understand which addresses are added where, from the documentation. In particular, what happens without --tags or --tagscc ? Also you should define `tag'; it has a lot of different meanings, some subtly different, and it is not completely clear which one you mean.
Is that because of the way I structured it? The actual behaviour Is stated under PROCESSING POLICY. Sure: I can explain what is read and what is done with the data.
I think you should formally state the default behaviour. Something like:
By default:
* get_maintainer is called on each patch to find email addresses of maintainers/reviewers for that patch; these are added to the patch body near the CC section.
* further email addresses are found in each patch's commit message tags (CC, signed-off-by, reviewed-by, etc.)
* All of the above addresses are added to the CC mail headers of each patch
* All of the above addresses are added to the CC mail headers of the cover letter
I suspect that what I have above is not the real behaviour. You should write what is true in that kind of style :-).
Sure. That makes things a lot clearer.
scripts/add_maintainers.pl line 180
Quoted 3 lines
scripts/add_maintainers.pl new file +3
-0+my $patch_prefix = "0"; # Use a 0, such that v* does not get picked up + # Obviously this will only work for series with + # < 999 patches, which should be fine
I don't understand the purpose of this:
This is a bit of a hack!
There are several different usage patterns for g-f-p when working on a series, which result in $patch_dir being used differently. In one case
a) the user stores patches for multiple series in $patch_dir
Thus, $patchdir may contain files starting with 0000*, 0001*, ... v1-000*, v2-000* I have directories that contain entries generated by
Case a1)
g-f-p ...
g-f-p --reroll-count=2 ...
Etc.
and
Case a2) g-f-p --reroll-count=1 ...
Etc.
b) the user stores patches in separate directories, aka one directory per g-f-p
What I was trying to do here is to use $patch_prefix to select what to process in $patchdir. The problem is that in case a1, when g-f-p was called with no --reroll-count, I need to select entries 0000*, 0001*, ... as otherwise the entire directory is processed.
The only way to identify these is via 0*.patch. But I may have missed something.
scripts/add_maintainers.pl line 225
Quoted 3 lines
scripts/add_maintainers.pl new file +3
-0+if ($rerollcount > 0) { + $patch_prefix = "v".$rerollcount; +}
...
> +my $pattern = $patch_dir.'/'.$patch_prefix.'*'.$patch_ext;
> +print "Then perform:\n".
> + "git send-email -to xen-devel\@lists.xenproject.org ".
> + $patch_dir.'/'.$patch_prefix."*.patch"."\n";
What files matching *.patch exist here that should not be processed ?
If the answer is none then $patch_prefix is redundant, I think ?Well, it depends. G-s-m will send everything in $patch_dir.
I have not checked whether it ignores backups (~ .bak), but I assume it does. In any case, for scenario a1) and a2) I do need to select which files to select in g-s-e.
scripts/add_maintainers.pl line 254
Quoted 2 lines
scripts/add_maintainers.pl new file +2
-0+foreach my $file (@patches) { + if ($file =~ /\Q$cover_letter\E/) {
I know you had me look at this over your shoulder and I said it was right, but I think in fact this would match hypothetical files
$patch_dir/0020-do-something-about-0000-cover-letter.patch
I think you need to expect a /. So one of
+ if ($file =~ /\/\Q$cover_letter\E/) {
+ if ($file =~ m{/\Q$cover_letter\E}) {
Sure
scripts/add_maintainers.pl line 333
Quoted 8 lines
scripts/add_maintainers.pl new file +8
-0+print "Then perform:\n". + "git send-email -to xen-devel\@lists.xenproject.org ". + $patch_dir.'/'.$patch_prefix."*.patch"."\n"; + +exit 0; + +my $readmailinglists = 0; +my @mailinglists = ();
This is a very curious structure. These assignments are never executed (but I guess the program works anyway). I would recommend moving the main program to the bottom of the file.
> +sub getmailinglists () {
> + # Read mailing list from MAINTAINERS file and copy
> + # a list of e-mail addresses to @mailinglists
> + if (!$readmailinglists) {
I suggest you rename this variable $getmailinglists_done or something. As it is it is confusing because `read' might be the present tense, but then the sense is wrong.
Sure
Also, you might find it better to use a structure like one of
if ($getmailingslists_done) { return; }
return if $getmailingslists_done;
> + if (-e $maintainers) {
...
> + print "Warning: Mailing lists will be treated as CC's\n";
> + }
> + # Don't try again, even if the MAINTAINERS file does not exist
> + $readmailinglists = 1;
> + # Remove any duplicates
> + @mailinglists = uniq @mailinglists;
> + }
Indentation here is misleading. (But this will go away if you adopt
my suggestion above).
> +sub ismailinglist ($) {
> + my ($check) = @_;
> + # Get the mailing list information
> + getmailinglists();
> + # Do the check
> + if ( grep { $_ eq $check} @mailinglists) {
Rather than uniq above, and then grep here, you could use a hash
%mailinglists. That would be more idiomatic and also less code and
faster. But as it is is tolerable.
> +sub getmaintainers ($$$) {
> + my ($file, $rto, $rcc) = @_;
> + my $get_maintainer_args = join " ", @get_maintainer_args;
> + my $cmd = "$get_maintainer $get_maintainer_args <$file";
...
> + open($fh, "-|", $cmd)
> + or die "Failed to open '$cmd'\n";
You should use the array form of piped open, rather than this string
joining. That way arguments containing spaces make their way through
correct.Will look into this.
scripts/add_maintainers.pl line 379
Quoted 2 lines
scripts/add_maintainers.pl new file +2
-0+ if (! -e $get_maintainer) { + die "$tool: The tool requires $get_maintainer\n";
I still don't like this check. What if the user specifies an implementation of $get_maintainer which is on the PATH ?
The way get_maintainer.pl works is that it has to be called in the root directory of the Xen and Linux trees. There are some checks in the tool that throw an error when you call it from another location.
The relevant code in get_maintainer.pl is
268 if (!top_of_tree($xen_path)) {
269 die "$P: The current directory does not appear to be "
270 . "a Xen source tree.\n";
271 }
Right now, $get_maintainer is not really configurable. So this is not an issue now, but could become one in future.
I think for now, I will leave this as-is, but add a comment.
Quoted 1 line
+ while(my $line = <$fh>) {
...
> + }
> + close $fh;
You need to check the errors here. See the `perldoc -f close'.OK
Quoted 1 line
+ if ($tags & !$tagscc) {
You mean &&, not &.
Indeed
Thanks for the review
Lars
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
Lars Kurth writes ("Re: [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"):
Quoted 5 lines
scripts/add_maintainers.pl line 180
Quoted 3 lines · Ian Jackson
scripts/add_maintainers.pl new file +3
-0+my $patch_prefix = "0"; # Use a 0, such that v* does not get picked up + # Obviously this will only work for series with + # < 999 patches, which should be fine
Quoted 8 lines
I don't understand the purpose of this:
This is a bit of a hack!
There are several different usage patterns for g-f-p when working on a series, which result in $patch_dir being used differently. In one case
a) the user stores patches for multiple series in $patch_dir
Thus, $patchdir may contain files starting with 0000*, 0001*, ... v1-000*, v2-000*
OMG. It had not even occurred to me that anyone would do that. But I think this workflow choice is independent of --reroll-count, which is mainly used to control patch subject lines.
A workflow where *different* patch series are mingled in the same directory cannot be supported, because what when their reroll counts collide? So these must be different versions (different rerolls) of the same series.
I suggest the following approach:
Test whether v<reroll-count>-0000-cover-letter.patch exists. If it does, expect every patch to start with v<reroll-count>. If it does not, expect simply 0000-cover-letter.patch to exist, and every patch to start with just the patch number.
So I guess $patch_prefix would be '[0-9]' or "v${reroll_count}-[0-9]".
Quoted 12 lines
scripts/add_maintainers.pl line 225
Quoted 3 lines
scripts/add_maintainers.pl new file +3
-0+if ($rerollcount > 0) { + $patch_prefix = "v".$rerollcount; +}... > +my $pattern = $patch_dir.'/'.$patch_prefix.'*'.$patch_ext; > +print "Then perform:\n". > + "git send-email -to xen-devel\@lists.xenproject.org ". > + $patch_dir.'/'.$patch_prefix."*.patch"."\n"; What files matching *.patch exist here that should not be processed ? If the answer is none then $patch_prefix is redundant, I think ?
Quoted 4 lines
Well, it depends. G-s-m will send everything in $patch_dir.
I have not checked whether it ignores backups (~ .bak), but I assume it does. In any case, for scenario a1) and a2) I do need to select which files to select in g-s-e.
For the purposes of documentation and informative messages like this one, I think you can assume workflow (b). I think workflow (a) is not to be recommended.
(Anyway, who keeps their g-f-p output directories ? I don't...)
Quoted 9 lines
scripts/add_maintainers.pl line 379
Quoted 2 lines
scripts/add_maintainers.pl new file +2
-0+ if (! -e $get_maintainer) { + die "$tool: The tool requires $get_maintainer\n";I still don't like this check. What if the user specifies an implementation of $get_maintainer which is on the PATH ?
The way get_maintainer.pl works is that it has to be called in the root directory of the Xen and Linux trees. There are some checks in the tool that throw an error when you call it from another location.
You have misunderstood my remark. I am not talking about the cwd.
I mean, if the user says --get-maintainer=generic-get-maintaienr where /usr/bin/generic-get-maintaienr exists.
Thanks,
Ian.
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
On 08/05/2018, 11:48, "Ian Jackson" <[email protected]> wrote:
Lars Kurth writes ("Re: [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 04/05/2018, 18:43, "Ian Jackson" <[email protected]> wrote:
> > +my $patch_prefix = "0"; # Use a 0, such that v* does not get picked up
> > + # Obviously this will only work for series with
> > + # < 999 patches, which should be fine
>
> I don't understand the purpose of this:
>
> This is a bit of a hack!
>
> There are several different usage patterns for g-f-p when working on a series,
> which result in $patch_dir being used differently. In one case
> a) the user stores patches for multiple series in $patch_dir
> Thus, $patchdir may contain files starting with 0000*, 0001*, ... v1-000*, v2-000*
OMG. It had not even occurred to me that anyone would do that. But I think this workflow choice is independent of --reroll-count, which is mainly used to control patch subject lines.
A workflow where *different* patch series are mingled in the same directory cannot be supported, because what when their reroll counts collide? So these must be different versions (different rerolls) of the same series.
I suggest the following approach:
Test whether v<reroll-count>-0000-cover-letter.patch exists. If it does, expect every patch to start with v<reroll-count>. If it does not, expect simply 0000-cover-letter.patch to exist, and every patch to start with just the patch number.
So I guess $patch_prefix would be '[0-9]' or "v${reroll_count}-[0-9]".
That makes sense
Quoted 12 lines
scripts/add_maintainers.pl line 225
Quoted 3 lines
scripts/add_maintainers.pl new file +3
-0+if ($rerollcount > 0) { + $patch_prefix = "v".$rerollcount; +}... > +my $pattern = $patch_dir.'/'.$patch_prefix.'*'.$patch_ext; > +print "Then perform:\n". > + "git send-email -to xen-devel\@lists.xenproject.org ". > + $patch_dir.'/'.$patch_prefix."*.patch"."\n"; What files matching *.patch exist here that should not be processed ? If the answer is none then $patch_prefix is redundant, I think ?
Quoted 4 lines
Well, it depends. G-s-m will send everything in $patch_dir.
I have not checked whether it ignores backups (~ .bak), but I assume it does. In any case, for scenario a1) and a2) I do need to select which files to select in g-s-e.
For the purposes of documentation and informative messages like this one, I think you can assume workflow (b). I think workflow (a) is not to be recommended.
(Anyway, who keeps their g-f-p output directories ? I don't...)
Sure: I think right now the documentation and messages don't cover this. So if we move to the approach you suggested, all should be fine.
Quoted 9 lines
scripts/add_maintainers.pl line 379
Quoted 2 lines
scripts/add_maintainers.pl new file +2
-0+ if (! -e $get_maintainer) { + die "$tool: The tool requires $get_maintainer\n";I still don't like this check. What if the user specifies an implementation of $get_maintainer which is on the PATH ?
The way get_maintainer.pl works is that it has to be called in the root directory of the Xen and Linux trees. There are some checks in the tool that throw an error when you call it from another location.
You have misunderstood my remark. I am not talking about the cwd.
I mean, if the user says --get-maintainer=generic-get-maintaienr where /usr/bin/generic-get-maintaienr exists.
OK: simply removing the check would just do this
Lars
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
Quoted 107 lines · Lars Kurth
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 [email protected] <patchdir>/*.patchxmI 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 ofindex()and replaced with regex
- Removed non-perl idioms
- Moved uniq statements to normalize and added info on what normalize does
- Read L: tags fromMAINTAINERSfile instead of using heuristic
- Fixed issues related to metacharacters ingetmaintainers()
- 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 reworkinginsert()
- 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 <[email protected]>Cc George Dunlap <[email protected]>Cc Ian Jackson <[email protected]>Cc Jan Beulich <[email protected]>Cc Julien Grall <[email protected]>Cc Konrad Rzeszutek Wilk <[email protected]>Cc Stefano Stabellini <[email protected]>Cc Tim Deegan <[email protected]>Cc Wei Liu <[email protected]>Signed-off-by Lars Kurth <[email protected]>Release-acked-by Juergen Gross <[email protected]>--- scripts/add_maintainers.pl | 512 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 512 insertions(+) create mode 100755 scripts/add_maintainers.plscripts/add_maintainers.pl new file +41
-0@@ -0,0 +1,512 @@ +#!/usr/bin/perl -w +# (c) 2018, Lars Kurth <[email protected]> +# +# 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:
scripts/add_maintainers.pl line 51
Quoted 10 lines
scripts/add_maintainers.pl new file +10
-0+ --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.
scripts/add_maintainers.pl line 60
Quoted 9 lines
scripts/add_maintainers.pl new file +9
-0+ --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.
scripts/add_maintainers.pl line 69
Quoted 9 lines
scripts/add_maintainers.pl new file +9
-0+------------------ + *.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.
Quoted 8 lines
scripts/add_maintainers.pl new file +8
-0+ <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)
scripts/add_maintainers.pl line 87
Quoted 10 lines
scripts/add_maintainers.pl new file +10
-0+ 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.
---
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
Quoted 8 lines · George Dunlap
--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)
s/top/header/g; for these
-George
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
Quoted 203 lines · George Dunlap
Quoted 107 lines · Lars Kurth
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 [email protected] <patchdir>/*.patchxmI 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 ofindex()and replaced with regex
- Removed non-perl idioms
- Moved uniq statements to normalize and added info on what normalize does
- Read L: tags fromMAINTAINERSfile instead of using heuristic
- Fixed issues related to metacharacters ingetmaintainers()
- 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 reworkinginsert()
- 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 <[email protected]>Cc George Dunlap <[email protected]>Cc Ian Jackson <[email protected]>Cc Jan Beulich <[email protected]>Cc Julien Grall <[email protected]>Cc Konrad Rzeszutek Wilk <[email protected]>Cc Stefano Stabellini <[email protected]>Cc Tim Deegan <[email protected]>Cc Wei Liu <[email protected]>Signed-off-by Lars Kurth <[email protected]>Release-acked-by Juergen Gross <[email protected]>--- scripts/add_maintainers.pl | 512 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 512 insertions(+) create mode 100755 scripts/add_maintainers.plscripts/add_maintainers.pl new file +41
-0@@ -0,0 +1,512 @@ +#!/usr/bin/perl -w +# (c) 2018, Lars Kurth <[email protected]> +# +# 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:scripts/add_maintainers.pl line 51
Quoted 10 lines
scripts/add_maintainers.pl new file +10
-0+ --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 fileNot clear on the difference between these.
scripts/add_maintainers.pl line 60
Quoted 9 lines
scripts/add_maintainers.pl new file +9
-0+ --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.
scripts/add_maintainers.pl line 69
Quoted 9 lines
scripts/add_maintainers.pl new file +9
-0+------------------ + *.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.
Quoted 8 lines
scripts/add_maintainers.pl new file +8
-0+ <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)scripts/add_maintainers.pl line 87
Quoted 10 lines
scripts/add_maintainers.pl new file +10
-0+ 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 CCsI 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.
Quoted 7 lines
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.
OK, having talked to you IRL I now understand what these are about. How about this:
---
--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.
For patches, these extra lines will be inserted as specified by the --patchcc option, *unless* that option is set to commit. In that case, rather than duplicate tags in the commit message with CC lines, the extra CC lines will be added to he header instead.
--tagscc
As above, but commit location is not special-cased: In the case of commit, all CCs will be added to the bottom of the commit message, even if they duplicate other tags in the commit message.
---
Or, perhaps even better:
---
--tags[=(split|combined)] | -t
In addition to the output of get_maintainer.pl, collect email addresses from commit tags (e.g., Reviewed-by, Tested-by, &c) in the list of CC lines to insert.
If 'split' is specified, these tag CC lines will be added to the header, rather than in the location specified by --patchcc. In the case that commit is specified, this will avoid duplicating tags and CC lines in the commit message. This is the default when --patchcc commit is specified.
If 'combined' is specified, these tag CC lines will be added along with the maintainer CC's, wherever specified by --patchcc. This is the default when "header" or "comment" are specified.
---
-George
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
On 10/05/2018, 14:03, "George Dunlap" <[email protected]> wrote:
On 05/10/2018 12:38 PM, George Dunlap 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 [email protected] <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 <[email protected]>
>> Cc: George Dunlap <[email protected]>
>> Cc: Ian Jackson <[email protected]>
>> Cc: Jan Beulich <[email protected]>
>> Cc: Julien Grall <[email protected]>
>> Cc: Konrad Rzeszutek Wilk <[email protected]>
>> Cc: Stefano Stabellini <[email protected]>
>> Cc: Tim Deegan <[email protected]>
>> Cc: Wei Liu <[email protected]>
>> Signed-off-by: Lars Kurth <[email protected]>
>> Release-acked-by: Juergen Gross <[email protected]>
>> ---
>> 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 <[email protected]>
>> +#
>> +# 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.
OK, having talked to you IRL I now understand what these are about. How about this:
---
--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.
For patches, these extra lines will be inserted as specified by the --patchcc option, *unless* that option is set to commit. In that case, rather than duplicate tags in the commit message with CC lines, the extra CC lines will be added to he header instead.
--tagscc
As above, but commit location is not special-cased: In the case of commit, all CCs will be added to the bottom of the commit message, even if they duplicate other tags in the commit message.
---
Or, perhaps even better:
---
--tags[=(split|combined)] | -t
In addition to the output of get_maintainer.pl, collect email addresses from commit tags (e.g., Reviewed-by, Tested-by, &c) in the list of CC lines to insert.
If 'split' is specified, these tag CC lines will be added to the header, rather than in the location specified by --patchcc. In the case that commit is specified, this will avoid duplicating tags and CC lines in the commit message. This is the default when --patchcc commit is specified.
If 'combined' is specified, these tag CC lines will be added along with the maintainer CC's, wherever specified by --patchcc. This is the default when "header" or "comment" are specified.
---
George: thanks for the feedback. I think that's an improvement.
Although the behaviour you describe is not quite correct as --tags will always insert any CC lines into the top of a mail aka simulating --tagscc=top and --covercc=top
, whereas
--tagscc follows the behaviour specified by --patchcc and --covercc
I don't mind renaming the options (and some of the code which uses the term policy in variable names, as that makes things clearer.
Lars
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
On 10/05/2018, 12:38, "George Dunlap" <[email protected]> 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 [email protected] <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 <[email protected]>
> Cc: George Dunlap <[email protected]>
> Cc: Ian Jackson <[email protected]>
> Cc: Jan Beulich <[email protected]>
> Cc: Julien Grall <[email protected]>
> Cc: Konrad Rzeszutek Wilk <[email protected]>
> Cc: Stefano Stabellini <[email protected]>
> Cc: Tim Deegan <[email protected]>
> Cc: Wei Liu <[email protected]>
> Signed-off-by: Lars Kurth <[email protected]>
> Release-acked-by: Juergen Gross <[email protected]>
> ---
> 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 <[email protected]>
> +#
> +# 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
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel