git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
To: Kim Thuat NGUYEN <kim-thuat.nguyen@ensimag.imag.fr>
Cc: git@vger.kernel.org, nguyenkimthuat <nguyenkimthuat@gmail.com>,
	VOLEK Pavel <Pavel.Volek@ensimag.imag.fr>,
	ROUCHER IGLESIAS Javier <roucherj@ensimag.imag.fr>
Subject: Re: [PATCHv1] Export file in git-remote-mediawiki
Date: Fri, 08 Jun 2012 16:07:23 +0200	[thread overview]
Message-ID: <vpqwr3hrj6s.fsf@bauges.imag.fr> (raw)
In-Reply-To: <1339162024-3120-1-git-send-email-nguyenkimthuat@gmail.com> (Kim Thuat NGUYEN's message of "Fri, 8 Jun 2012 15:27:04 +0200")

> Subject: Re: [PATCHv1] Export file in git-remote-mediawiki

We usually write commit subject lines as "subsystem: description", hence

git-remote-mediawiki: export "File:" attachments

Kim Thuat NGUYEN <kim-thuat.nguyen@ensimag.imag.fr> writes:

> From: nguyenkimthuat <nguyenkimthuat@gmail.com>
>
> This patch adds the functionnality to export the file attachements from the local git's repository using the API of mediawiki. It also provides the possibility for
> an user to delete a page in the local repository Git which means the page  will be deleted in the wiki site after user do the 'push'.

Please, avoid long lines (> 80 characters).

> +	open(my $g,"-|","git " . $_[0]); 

Space after , please.

> +	my %hashFiles = get_file_extensions_maybe($complete_file_name);

What does this function do? My first understanding was that it queried
the wiki for allowed file extensions, but why does it need the file
name? It does nothing if $complete_file_name ends with .mw, but then why
do you run it before entering the following if() statement?

>  	if (substr($complete_file_name,-3) eq ".mw") {
>  		my $title = substr($complete_file_name,0,-3);

> @@ -653,39 +666,74 @@ sub mw_push_file {
>  			# special priviledges. A common
>  			# convention is to replace the page
>  			# with this content instead:
> -			$file_content = DELETED_CONTENT;
> +			mw_connect_maybe();
> +			my $re = $mediawiki->edit( {
> +				action => 'delete',
> +				title => $title,
> +				reason => $summary 
> +				} )|| die $mediawiki-> {error}->{code} . ':' . $mediawiki->{error}->{details};

This is an unrelated topic, and should not appear in this patch.

If you want to propagate page deletions, then you also need to deal with
the case where the user is not allowed to do so (very common on
MediaWiki). Also, if you change the code corresponding to the comment
right above, you should update the comment too.

> +	elsif (exists($hashFiles{$extension}))      
> +	{

Brace on the same line as else please.

> +			} else {
> +				print STDERR "Empty file. Can not upload \n ";
> +				}

Broken indentation.

>  	} else {
>  		print STDERR "$complete_file_name not a mediawiki file (Not pushable on this version of git-remote-mediawiki).\n"
>  	}

Isn't the very point of this patch to remove this error message?

> @@ -825,3 +873,25 @@ sub mw_push_revision {
>  	print STDOUT "ok $remote\n";
>  	return 1;
>  }
> +
> +sub get_file_extensions_maybe {
> +	my $file_name = shift;
> +	my $est_mw_page = substr($file_name,-3) eq ".mw";

English please. "est" is french ;-).

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

  reply	other threads:[~2012-06-08 14:07 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-08 13:27 [PATCHv1] Export file in git-remote-mediawiki Kim Thuat NGUYEN
2012-06-08 14:07 ` Matthieu Moy [this message]
2012-06-08 22:59   ` nguyenki
2012-06-10 13:01     ` Matthieu Moy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=vpqwr3hrj6s.fsf@bauges.imag.fr \
    --to=matthieu.moy@grenoble-inp.fr \
    --cc=Pavel.Volek@ensimag.imag.fr \
    --cc=git@vger.kernel.org \
    --cc=kim-thuat.nguyen@ensimag.imag.fr \
    --cc=nguyenkimthuat@gmail.com \
    --cc=roucherj@ensimag.imag.fr \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).