From: Ted Zlatanov <tzz@lifelogs.com>
Date: Tue, 08 Oct 2013 20:12:56 GMT
Subject: Re: [PATCH] git-credential-netrc: fix uninitialized warning
Message-ID: <8761t7v7w7.fsf@flea.lifelogs.com>
In-Reply-To: <20131008200235.GG9464@google.com>

On Tue, 8 Oct 2013 13:02:35 -0700 Jonathan Nieder <jrnieder@gmail.com> wrote: 

JN> Ted Zlatanov wrote:
>> On Tue, 8 Oct 2013 12:41:47 -0700 Jonathan Nieder <jrnieder@gmail.com> wrote: 
JN> Ted Zlatanov wrote:

>>>> Simple patch to avoid unitialized warning and log what we'll do.
JN> Sign-off?
>> 
>> I didn't realize it was a requirement, must I?

JN> See Documentation/SubmittingPatches, section '(5) Sign your work'
JN> for what this means.

JN> If you just forgot to sign off, that's fine and I can forge it or go
JN> without.  If you are unable to sign off because you don't have the
JN> right to submit the change under an open source license, I'd be a bit
JN> worried going forward.

Right, I got it.  Sorry, I didn't know that applied to trivial patches.

JN> After this patch, the code looks like

JN> 	if (!defined $entry->{$check}) {
JN> 		log_debug(...);
JN> 	} elsif (defined $query->{$check}) {
JN> 		...
JN> 	} else {
JN> 		log_debug(...);
JN> 	}

JN> As a small nit, wouldn't it be more readable with the two !defined()
JN> cases together?

JN> 	if (!defined $entry->{$check}) {
JN> 		...
JN> 	} elsif (!defined $query->{$check}) {
JN> 		...
JN> 	} else {
JN> 		...
JN> 	}

Because of way the "next ENTRY" line comes out, I like my way slightly
better, but honestly it's fine either way :)  If you like, go ahead and
commit the rewrite the way it works for you, I have no objections at all.

Ted