Re: [PATCH v2 02/22] git-remote-mediawiki: Use the Readonly module instead of the constant pragma

From: Benoît Person <benoit.person@ensimag.fr>
Date: Jun 8, 2013, 08:51 UTC
Message-ID: <CAETqRCia1CqkaaHCFk=7mxVgmuN1VhBbGVs_oEaObUxwHZ33Zw@mail.gmail.com>
In-Reply-To: <20130608032324.GA20226@sigill.intra.peff.net>

The major drawback of using perl `constant` is the fact that they do not interpolate like variables in most of the contexts (those who automatically quotes barewords). Like said on Perl Monks here [1], you have to do some "tricks" depending on the context in which you're using the constant and that's not really "clean".

But maybe trading clean for another package is not worth it. I just searched for alternatives way of doing it and none seems to be in the core packages so maybe we should just drop this.

[1] http://www.perlmonks.org/?node_id=777711

On 8 June 2013 05:23, Jeff King <peff@peff.net> wrote:

Show 29 quoted lines

> On Fri, Jun 07, 2013 at 11:42:03PM +0200, Célestin Matte wrote:
>
>> diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl b/contrib/mw-to-git/git-remote-mediawiki.perl
>> index 4893442..e60793a 100755
>> --- a/contrib/mw-to-git/git-remote-mediawiki.perl
>> +++ b/contrib/mw-to-git/git-remote-mediawiki.perl
>> @@ -26,21 +26,21 @@ use IPC::Open2;
>>  use Readonly;
>
> What does this series apply on top of? The existing version in "master"
> does not have "use Readonly" in it at all. The first version of your
> series introduced that line, but here it is shown as an existing line.
> Did you miss a commit when putting your patches together?
>
>>  # Mediawiki filenames can contain forward slashes. This variable decides by which pattern they should be replaced
>> -use constant SLASH_REPLACEMENT => "%2F";
>> +Readonly my $SLASH_REPLACEMENT => "%2F";
>
> What advantage does this have over "use constant"? I do not mind
> following guidelines from perlcritic if they are a matter of style, but
> in this case there is a cost: we now depend on the "Readonly" module,
> which is not part of the standard distribution. I.e., users now have to
> deal with installing an extra dependency. Is it worth it?
>
> -Peff
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html