From: Andreas Heiduk <asheiduk@gmail.com>
To: Eric Sunshine <sunshine@sunshineco.com>
Cc: Git Mailing List <git@vger.kernel.org>, Eric Wong <e@80x24.org>
Subject: Re: [PATCH 2/2] git-svn: allow empty email-address in authors-prog and authors-file
Date: Tue, 6 Mar 2018 23:24:50 +0100 [thread overview]
Message-ID: <426e019a-cfb2-0e3a-b9b4-9d94f4f79312@gmail.com> (raw)
In-Reply-To: <CAFhHFBysKuDO9H4yJtnC6MJ+Jih5q4RsfwHTCsRXhXknapp4xg@mail.gmail.com>
Am 05.03.2018 um 10:37 schrieb Andreas Heiduk:
> 2018-03-05 2:42 GMT+01:00 Eric Sunshine <sunshine@sunshineco.com>:
>> On Sun, Mar 4, 2018 at 6:22 AM, Andreas Heiduk <asheiduk@gmail.com> wrote:
>>> ---
>>> diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
>>> @@ -1482,7 +1482,6 @@ sub call_authors_prog {
>>> }
>>> if ($author =~ /^\s*(.+?)\s*<(.*)>\s*$/) {
>>> my ($name, $email) = ($1, $2);
>>> - $email = undef if length $2 == 0;
>>> return [$name, $email];
>>
>> Mental note: existing behavior intentionally makes $email undefined if
>> not present in $author; revised behavior leaves it defined.
>
> But only if the data comes from authors-prog. authors-file is unaffected.
>
>>
>>> } else {
>>> die "Author: $orig_author: $::_authors_prog returned "
>>> @@ -2020,8 +2019,8 @@ sub make_log_entry {
>>> remove_username($full_url);
>>> $log_entry{metadata} = "$full_url\@$r $uuid";
>>> $log_entry{svm_revision} = $r;
>>> - $email ||= "$author\@$uuid";
>>> - $commit_email ||= "$author\@$uuid";
>>> + $email //= "$author\@$uuid";
>>> + $commit_email //= "$author\@$uuid";
>>
>> With the revised behavior (above), $email is unconditionally defined,
>> so these //= expressions will _never_ assign "$author\@$uuid" to
>> $email. Am I reading that correctly? If so, then isn't this now just
>> dead code? Wouldn't it be clearer to remove these lines altogether?
>
> The olf behaviour still kicks in if
> - neither authors-file nor authors-prog is used
> - only authors-file is used
>
>> I see from reading the code that there is a "if (!defined $email)"
>> earlier in the function which becomes misleading with this change. I'd
>> have expected the patch to modify that, as well.
>
> I will look into that one later.
I don't want to let that slip through the cracks: The `if` statement
still makes a difference if:
- neither ` --authors-prog` nor `--authors-file` is active,
- but `--use-log-author` is active and
- the commit at hand does not contain a `From:` or `Signed-off-by:`
trailer.
In that case the result was and still is `$user <$user>` for the author
and `$user <$user@$uuid>` for the comitter. That doesn't make sense to
me but doesn't concern me right now.
next prev parent reply other threads:[~2018-03-06 22:24 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-03-04 11:22 [PATCH 1/2] git-svn: search --authors-prog in PATH too Andreas Heiduk
2018-03-04 11:22 ` [PATCH 2/2] git-svn: allow empty email-address in authors-prog and authors-file Andreas Heiduk
2018-03-05 1:42 ` Eric Sunshine
2018-03-05 9:37 ` Andreas Heiduk
2018-03-05 20:20 ` Eric Wong
2018-03-05 22:22 ` Eric Sunshine
2018-03-06 22:24 ` Andreas Heiduk [this message]
2018-03-05 0:52 ` [PATCH 1/2] git-svn: search --authors-prog in PATH too Eric Sunshine
2018-03-05 17:52 ` Eric Wong
2018-03-05 19:48 ` Andreas Heiduk
2018-03-05 20:16 ` Andreas Heiduk
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=426e019a-cfb2-0e3a-b9b4-9d94f4f79312@gmail.com \
--to=asheiduk@gmail.com \
--cc=e@80x24.org \
--cc=git@vger.kernel.org \
--cc=sunshine@sunshineco.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).