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

[Ian Jackson <ian.jackson@xxxxxxxxxx>]

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" <ian.jackson@xxxxxxxxxx> 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]".

>     > +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.

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...)

>     > +    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
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel