From: Wincent Colaiuta <win@wincent.com>
Date: Thu, 22 Nov 2007 10:07:25 GMT
Subject: Re: [PATCH] Authenticate only once in git-send-email
Message-ID: <C0E02CFE-F6E6-4902-8BA1-D6AC02F1C04F@wincent.com>
In-Reply-To: <7vwssa7jii.fsf@gitster.siamese.dyndns.org>

El 22/11/2007, a las 9:48, Junio C Hamano escribió:

> Wincent Colaiuta <win@wincent.com> writes:
>
>> This commit teaches git-send-email to authenticate once and only  
>> once at
>> the beginning of the series.
>
> Ok.  What does $smtp->auth() return?  Presumably a true value,

True on success, false on failure.

> but I do not find it the best coding style to hide a call made
> primarily for its effects not for its return value behind a
> conditional assignment to a boolean.  Eek.
>
>> 		if ((defined $smtp_authuser) && (defined $smtp_authpass)) {
>> -			$smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp- 
>> >message;
>> +			$auth ||= $smtp->auth( $smtp_authuser, $smtp_authpass ) or die  
>> $smtp->message;
>> 		}
>
> Perhaps something along the lines of...
>
>> 		if ((defined $smtp_authuser) && (defined $smtp_authpass)) {
>> 			$smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp- 
>> >message;
>> +			undef $smtp_authpass;
>> 		}
>
> ... or using a separate boolean variable "my $auth_happened"
> may be more appropriate.

Of the alternatives you suggest, I think an "$auth_happened" or  
"$auth_done" flag is probably the nicest/cleanest.

> But I am just saying this; I do not care _too_ deeply about it.
> Will apply as-is.

Yes, I don't really care either. My reasoning for doing it the way I  
did is that I'm not much of a Perl hacker so I basically wanted to  
make the change as minimally invasive as possible (in this case  
prepending "$auth ||= "). But above all, all that I really care about  
is that the problem gets fixed.

Cheers,
Wincent