bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 1/2] gitlog-to-changelog: support multi-author commits.


From: Jim Meyering
Subject: Re: [PATCH 1/2] gitlog-to-changelog: support multi-author commits.
Date: Wed, 16 Nov 2011 21:01:06 +0100

Gary V. Vaughan wrote:
...
> the parts that didn't work OOTB on my Mac to be portable.  Feel free to crib
> those portable parts of this one into coreutils, or reformat this one to
> coreutils style as you prefer.
>
>> An alternative is to accept anything after the ":" and then, to use
>> "s/\s*</  </" to ensure that the number of spaces before the "<" is two.
>>
>>> +      s/^Co-authored-by:\s*([^<]+)\s+</\t    \1  </
>>
>> Please use $1, not \1.
>
> I thought $1 was a positional parameter?  I'm not sure what the advantage of
> choosing a different back-reference syntax to other unix regexp using tools
> is, but no matter... after taking your other suggestions, this line doesn't
> exist any more :)

$1 is preferred in this context.
I'm pretty sure Perl style guides say that, too.

...
> Okay to push the following?

Almost.

> From 73131d956d8da994bae5f59e321548ba26ddd566 Mon Sep 17 00:00:00 2001
> From: "Gary V. Vaughan" <address@hidden>
> Date: Tue, 1 Nov 2011 17:58:37 +0700
> Subject: [PATCH 1/2] gitlog-to-changelog: support multi-author commits.
...
> +      my @coauthors = grep /^Co-authored-by:.*$/, @line;
> +      for (@coauthors)
> +        {
> +          s/^Co-authored-by:\s*/\t    /;
> +          s/\s*</  </;
> +
> +          $_ =~ /<address@hidden>/
> +            or warn "$ME: warning: Missing email address for "
> +              . substr ($_, 5) . ".\n";

There's no point in using $_ there.
You didn't just above, either, so removing it makes this consistent.
(preferred style, too, though in 95% of foreach loops I write I do
not use $_, due to the risk -- didn't seem like worth trying to
work around in this case)

I also changed the first ".*" to ".*?", mostly on reflex.

No capital "M", and no period at end of diagnostic.

Apply the following and you may push the result,
but without the commit-msg script addition.

diff --git a/build-aux/gitlog-to-changelog b/build-aux/gitlog-to-changelog
index 9d98b82..f5f6023 100755
--- a/build-aux/gitlog-to-changelog
+++ b/build-aux/gitlog-to-changelog
@@ -259,9 +259,9 @@ sub parse_amend_file($)
           s/^Co-authored-by:\s*/\t    /;
           s/\s*</  </;

-          $_ =~ /<address@hidden>/
-            or warn "$ME: warning: Missing email address for "
-              . substr ($_, 5) . ".\n";
+          /<address@hidden>/
+            or warn "$ME: warning: missing email address for "
+              . substr ($_, 5) . "\n";
         }

       # If this header would be the same as the previous date/name/email/

...
> diff --git a/scripts/git-hooks/commit-msg b/scripts/git-hooks/commit-msg
> new file mode 100755
> index 0000000..5efdf1f
> --- /dev/null
> +++ b/scripts/git-hooks/commit-msg
> @@ -0,0 +1,85 @@
> +#!/bin/sh
> +# An example hook script for catching duplicate or malformed
> +# Co-authored-by lines in the commit message.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]