Re: [RFC PATCH 1/2] add a local copy of Mail::Address from CPAN

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Jan 5, 2018, 14:19 UTC
Message-ID: <87shbkbd6r.fsf@evledraar.gmail.com>
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.