From: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date: Mon, 11 Jun 2012 20:38:36 GMT
Subject: Re: [PATCHv2 1/2] git-remote-mediawiki: import "File:" attachments
Message-ID: <vpq7gvd1t4j.fsf@bauges.imag.fr>
In-Reply-To: <1339442945-8561-1-git-send-email-Pavel.Volek@ensimag.imag.fr>

Pavel Volek <Pavel.Volek@ensimag.imag.fr> writes:

> +		# Get pages of related media files.
> +		if ($import_media) {
> +			get_mw_pages_for_linked_mediafiles(\@tracked_pages, \%pages);
> +		}

The comment is useless given the function name.

> +# Returns MediaWiki id for a canonical namespace name.
> +# Ex.: "File", "Project".
> +# Looks for the namespace id in the local configuration
> +# variables, if it is not found asks MW API.

Functions are usually specified in imperative form, hence "Return", not
"Returns" for example.

> +	my $file = $mediawiki->download( { title => $filename } );

I'd call that $file_content, to avoid confusion with the file name.

Other than that, the patch looks good (but I didn't review very
carefully).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/