From: Ævar Arnfjörð Bjarmason Date: Fri, 05 Jan 2018 14:19:40 GMT Subject: Re: [RFC PATCH 1/2] add a local copy of Mail::Address from CPAN 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.