Re: [PATCH v3 07/28] git-remote-mediawiki: Rewrite unclear line of instructions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Célestin Matte <celestin.matte@xxxxxxxxxx> writes:

> Subroutines' parameters should be affected to variable before doing anything
> else
> Besides, existing instruction affected a variable inside a "if", which break
> Git's coding style

I think s/affect/assign/g is what you meant.

By the way, I often see two styles of the "let's take arguments into
parameters before doing anything else" at the beginning of subs:

        my ($namespace) = @_;
	my $namespace = shift;

My impression has been that both are equally common, but the latter
is done more often when picking out small and fixed number of
mandatory parameters upfront (and later, optional parameters are
used by directly reading what remains in @_).  Does Perlcritique say
anything about this issue?

> Signed-off-by: Célestin Matte <celestin.matte@xxxxxxxxxx>
> Signed-off-by: Matthieu Moy <matthieu.moy@xxxxxxxxxxxxxxx>
> ---
>  contrib/mw-to-git/git-remote-mediawiki.perl |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl b/contrib/mw-to-git/git-remote-mediawiki.perl
> index 431e063..2db6467 100755
> --- a/contrib/mw-to-git/git-remote-mediawiki.perl
> +++ b/contrib/mw-to-git/git-remote-mediawiki.perl
> @@ -1333,7 +1333,8 @@ sub get_mw_namespace_id {
>  }
>  
>  sub get_mw_namespace_id_for_page {
> -	if (my ($namespace) = $_[0] =~ /^([^:]*):/) {
> +	my $namespace = shift;
> +	if ($namespace =~ /^([^:]*):/) {
>  		return get_mw_namespace_id($namespace);
>  	} else {
>  		return;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]