Re: [PATCH 01/18] Follow perlcritic's recommendations - level 5 and 4
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Jun 7, 2013, 01:42 UTC
- Message-ID
- <CAPig+cTm0yXRdHgJR91qNwari08ZyZJjGTedGCqaO3zc3A06Ug@mail.gmail.com>
- In-Reply-To
- <1370547263-13558-2-git-send-email-celestin.matte@ensimag.fr>
On Thu, Jun 6, 2013 at 3:34 PM, Célestin Matte <celestin.matte@ensimag.fr> wrote:
Show 5 quoted lines
> Fix warnings from perlcritic's level 5 and 4. They correspond to the following > cases: > - always end a submodule with a return > - don't use the constant pragma, use the Readonly module instead > - some syntax details for maps, and others.
Although loosely related by being mentioned by perlcritic (4,5), each bullet point is otherwise unrelated, and mixing such unrelated changes into a single patch can make review more difficult.
Show 21 quoted lines
> Signed-off-by: Célestin Matte <celestin.matte@ensimag.fr> > Signed-off-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr> > --- > contrib/mw-to-git/git-remote-mediawiki.perl | 81 +++++++++++++++++---------- > 1 file changed, 51 insertions(+), 30 deletions(-) > > diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl b/contrib/mw-to-git/git-remote-mediawiki.perl > index 410eae9..83cf292 100755 > --- a/contrib/mw-to-git/git-remote-mediawiki.perl > +++ b/contrib/mw-to-git/git-remote-mediawiki.perl > @@ -15,32 +15,32 @@ use strict; > use MediaWiki::API; > use Git; > use DateTime::Format::ISO8601; > +use warnings; > > # By default, use UTF-8 to communicate with Git and the user > -binmode STDERR, ":utf8"; > -binmode STDOUT, ":utf8"; > +binmode STDERR, ":encoding(UTF-8)"; > +binmode STDOUT, ":encoding(UTF-8)";
This change isn't explained or rationalized in the commit message.
Show 6 quoted lines
> @@ -96,6 +96,9 @@ unless ($fetch_strategy) {
> $fetch_strategy = "by_page";
> }
>
> +# Remember the timestamp corresponding to a revision id.
> +my %basetimestamps;Although this is a simple textual relocation, it's not clear why it's needed or preferable, and the commit message does not explain it.
Show 16 quoted lines
> @@ -473,9 +486,6 @@ sub get_last_local_revision {
> return $lastrevision_number;
> }
>
> -# Remember the timestamp corresponding to a revision id.
> -my %basetimestamps;
> -
> # Get the last remote revision without taking in account which pages are
> # tracked or not. This function makes a single request to the wiki thus
> # avoid a loop onto all tracked pages. This is useful for the fetch-by-rev
> @@ -555,7 +565,7 @@ sub mediawiki_smudge {
>
> sub mediawiki_clean_filename {
> my $filename = shift;
> - $filename =~ s/@{[SLASH_REPLACEMENT]}/\//g;
> + $filename =~ s{$SLASH_REPLACEMENT}{/}g;Although patch 2/18 replaces regex // with {}, the change sneaked into this patch (1/18) prematurely.
Show 9 quoted lines
> # [, ], |, {, and } are forbidden by MediaWiki, even URL-encoded.
> # Do a variant of URL-encoding, i.e. looks like URL-encoding,
> # but with _ added to prevent MediaWiki from thinking this is
> @@ -569,7 +579,7 @@ sub mediawiki_clean_filename {
>
> sub mediawiki_smudge_filename {
> my $filename = shift;
> - $filename =~ s/\//@{[SLASH_REPLACEMENT]}/g;
> + $filename =~ s{/}{$SLASH_REPLACEMENT}g;Ditto regarding // to {}.Show 9 quoted lines
> $filename =~ s/ /_/g;
> # Decode forbidden characters encoded in mediawiki_clean_filename
> $filename =~ s/_%_([0-9a-fA-F][0-9a-fA-F])/sprintf("%c", hex($1))/ge;
> @@ -588,7 +599,8 @@ sub literal_data_raw {
> utf8::downgrade($content);
> binmode STDOUT, ":raw";
> print STDOUT "data ", bytes::length($content), "\n", $content;
> - binmode STDOUT, ":utf8";
> + binmode STDOUT, ":encoding(UTF-8)";Unexplained change.
Show 11 quoted lines
> + return;
> }
>
> sub mw_capabilities {
> @@ -1314,7 +1334,8 @@ sub get_mw_namespace_id {
> }
>
> sub get_mw_namespace_id_for_page {
> - if (my ($namespace) = $_[0] =~ /^([^:]*):/) {
> + my $namespace = shift;
> + if ($namespace =~ /^([^:]*):/) {Another change not mentioned by the commit message.
> return get_mw_namespace_id($namespace);
> } else {
> return;
> --