Re: [PATCH v2] add git credential login to remote mediawiki

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

 



Javier.Roucher-Iglesias@xxxxxxxxxxxxxxx writes:

> --- a/contrib/mw-to-git/git-remote-mediawiki
> +++ b/contrib/mw-to-git/git-remote-mediawiki
> @@ -154,26 +154,107 @@ while (<STDIN>) {
>  # MediaWiki API instance, created lazily.
>  my $mediawiki;
>  
[...]
> +my $mediawiki;

You're adding a second declaration of $mediawiki. Cut-and-paste issue?

Plus, didn't I already mention that this "my $mediawiki;" was meant to
be right above mw_connect_maybe?

> +        if ($wiki_login ne "") {
> +                $msg .= "username=$wiki_login\n";
> +        }

Indentation with space.

> +	my $key;
> +	my $value;
> +	my $Prog = "git credential $op";
> +	open2(*Reader, *Writer, $Prog);
> +	print Writer $msg;
> +	close (Writer);

No space before "(" (already mentionned off-list).

> +			# error if key undef
> +			if (not defined $key) {

The comment is useless and therefore counter-productive. Remove it.

> +				print STDERR "ERROR reciving reponse git credential fill\n";

You can add $_ to the message. If this ever happens, the user will
appreciate to see what's going on.

>  sub mw_connect_maybe {
> +
>  	if ($mediawiki) {

Don't add useless newlines.

> +		if (!$wiki_passwd) {
> +			#user knows, password not.
> +			ask_login();
>  		} else {
> -			print STDERR "Logged in with user \"$wiki_login\".\n";
> +			#user and password knows.
> +			ask_login();
>  		}

What is this? You have an "if" with both branches identical.

If the user didn't specify any login name, then you should try to
connect to the wiki anonymously, which works in many wikis.

(BTW, doesn't "ask_login" ask for a password more than a login? If so,
please rename the function).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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]