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

From: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date: Jun 12, 2012, 15:21 UTC
Message-ID: <vpqsje0mu7o.fsf@bauges.imag.fr>
In-Reply-To: <303c1249341d6f305d315b0a8d9cd20c@telesun.imag.fr>

roucherj <roucherj@telesun.imag.fr> writes:

Show 10 quoted lines

> On Tue, 12 Jun 2012 16:51:18 +0200, Matthieu Moy wrote:
>> Javier.Roucher-Iglesias@ensimag.imag.fr writes:
>>
>>>  # MediaWiki API instance, created lazily.
>>> +sub run_credential {
>>
>> How is the code related to the comment right above?
>>
>
> Sorry a old comment, i think it's a cause of git rebase i will erase.

The comment was already there in the code, but it was right above

my $mediawiki;

and therefore meaningful.

Show 5 quoted lines

> sorry but before it's like:
> +	my $Prog = "git credential $op";
> +	open2(*Reader, *Writer, $Prog);
> +	print Writer $msg;
> +	close(Writer);

No it wasn't (this is the expected version). It was "close (Writer);" with an extra space.

Show 5 quoted lines

>> Didn't I already mention (twice?) that this declaration was meant to
>> stay right above mw_connect_maybe?
>
> yes but 'ask_credential' used $mediawiki then i think i have to
> declare before 'ask_credential'

But do you have any reason not to declare ask_credential after mw_connect_maybe?

Show 16 quoted lines

>>> +			run_credential("reject");
>>> +#			exit 1;
>>
>> Do you, or do you not want to "exit 1". Either remove this, or
>> uncomment
>> it.
>>
>
> i have comment this line to see the opinion of the community, because
> i ask all the time to authenticate
> but there are few operations who we don't have to authenticate it.
>
> now if we comment this line the login will fail but they will continue
> with the operation (like, git clone)
> and try to do it, if the wiki need to be authenticate will refuse the
> operation if not they will continue.

[...]

Show 5 quoted lines

>> This means you can't use the wiki anonymously anymore. This is an
>> unacceptable regression.
>
> yes you can use anonymously but login will fail and try to the
> operation, this is the reason because i have comment the 'exit 1'.

But in this case, you still prompt for login and password, right?. That's weird for the user to have to type a dummy login/password and see an error message to use a wiki anonymously.

Show 9 quoted lines

>>>  sub mw_connect_maybe {
>>> +
>>>  	if ($mediawiki) {
>>
>> Why do you add this blank line? (already mentionned)
>>
>
> because we need the structure $mediawiki to get the url if is not
> declare it will return with out doing the operation

I'm talking about the blank line, not the if.

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