git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Matthieu Moy <git@matthieu-moy.fr>
Cc: git@vger.kernel.org, gitster@pobox.com,
	"Alex Bennée" <alex.bennee@linaro.org>,
	"Thomas Adam" <thomas@xteddy.org>
Subject: Re: [RFC PATCH 1/2] add a local copy of Mail::Address from CPAN
Date: Fri, 05 Jan 2018 15:19:40 +0100	[thread overview]
Message-ID: <87shbkbd6r.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <1515092151-14423-1-git-send-email-git@matthieu-moy.fr>


On Thu, Jan 04 2018, Matthieu Moy jotted:

> I looked at the perl/Git/Error.pm wrapper, and ended up writting a
> different, much simpler version. I'm not sure the same approach would
> apply to Error.pm, but my straightforward version does the job for
> Mail/Address.pm.

Yeah, yours is much simpler because Mail::Address doesn't have an import
method, which is the entire complexity in the Error.pm wrapper.

I'll probably submit a wrapper-for-the-wrappers patch at some point
after this gets in, i.e. both of these would become:

    package Git::Error;
    use Git::WrapCPAN 'Error';
    1;

    package Git::Mail::Address;
    use Git::WrapCPAN 'Mail::Address';
    1;

Then the Git::WrapCPAN package would do all the magic the Git::Error
wrapper is doing now, but with a configurable package.

But this doesn't have to wait for that.

> I would also be fine with using our local copy unconditionaly.

Some notes on your patch:

 * If I comment out your whole eval/or-do I was puzzled because tests
   still pass, turns out the eval { require Email::Valid } will bring in
   Mail::Address from CPAN.

   Not a bug, just something to note for others poking at this.

 * You didn't update t/t9000/test.pl to use the wrapper, which I thought
   was a bug until I realized this is built on top of
   gitster/mm/send-email-fallback-to-local-mail-address.

  parent reply	other threads:[~2018-01-05 14:19 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-04 18:55 [RFC PATCH 1/2] add a local copy of Mail::Address from CPAN Matthieu Moy
2018-01-04 18:55 ` [RFC PATCH 2/2] Remove now useless email-address parsing code Matthieu Moy
2018-01-04 22:11   ` Alex Bennée
2018-01-05  9:39     ` Matthieu Moy
2018-01-05 10:11       ` [PATCH] send-email: add test for Linux's get_maintainer.pl Matthieu Moy
2018-01-05 11:15         ` Alex Bennée
2018-01-05 11:36           ` Matthieu Moy
2018-01-05 20:16             ` Junio C Hamano
2018-01-04 21:02 ` [RFC PATCH 1/2] add a local copy of Mail::Address from CPAN Eric Sunshine
2018-01-05 14:19 ` Ævar Arnfjörð Bjarmason [this message]
2018-01-05 18:36 ` [PATCH v2 1/3] send-email: add and use a local copy of Mail::Address Matthieu Moy
2018-01-05 18:36   ` [PATCH v2 2/3] Remove now useless email-address parsing code Matthieu Moy
2018-01-05 18:36   ` [PATCH v2 3/3] send-email: add test for Linux's get_maintainer.pl Matthieu Moy
2018-01-05 18:59     ` Eric Sunshine
2018-01-08 10:30       ` Matthieu Moy
2018-01-08 10:34   ` [PATCH v3 1/3] send-email: add and use a local copy of Mail::Address Matthieu Moy
2018-01-08 10:34     ` [PATCH v3 2/3] Remove now useless email-address parsing code Matthieu Moy
2018-01-08 11:57       ` Alex Bennée
2018-01-08 10:34     ` [PATCH v3 3/3] send-email: add test for Linux's get_maintainer.pl Matthieu Moy
2018-01-08 18:45       ` Junio C Hamano
2018-01-08 11:56     ` [PATCH v3 1/3] send-email: add and use a local copy of Mail::Address Alex Bennée
2018-02-14 14:59   ` [PATCH v2 " Ævar Arnfjörð Bjarmason

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=87shbkbd6r.fsf@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=alex.bennee@linaro.org \
    --cc=git@matthieu-moy.fr \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=thomas@xteddy.org \
    /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).