On Tue, 8 Oct 2013 12:41:47 -0700 Jonathan Nieder [off-list ref] wrote:
JN> Ted Zlatanov wrote:
quoted
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> [...]
quoted
--- a/contrib/credential/netrc/git-credential-netrc
+++ b/contrib/credential/netrc/git-credential-netrc
@@ -369,7 +369,10 @@ sub find_netrc_entry {{
my $entry_text = join ', ', map { "$_=$entry->{$_}" } keys %$entry;
foreach my $check (sort keys %$query) {
- if (defined $query->{$check}) {
+ if (!defined $entry->{$check}) {
+ log_debug("OK: entry has no $check token, so any value satisfies check $check");
+ }
+ elsif (defined $query->{$check}) {
JN> Style: elsewhere this file seems to use cuddled elses:
JN> } elsif (...) {
Ah, thanks, I missed that.
JN> Or more simply, would it make sense to wrap both 'defined' checks into
JN> a single "if", like so?
JN> if (defined $entry->{$check} && defined $query->{$check}) {
JN> ...
JN> } else {
JN> log_debug(...);
JN> }
I prefer the explicit version because we can issue a more precise
log_debug message.
Ted
On 10/08/2013 09:55 PM, Ted Zlatanov wrote:
JN> Sign-off?
I didn't realize it was a requirement, must I?
Yes, this is a requirement. See Documentation/SubmittingPatches
to read what signing off actually means here.
Stefan
Ted Zlatanov wrote:
On Tue, 8 Oct 2013 12:41:47 -0700 Jonathan Nieder [off-list ref] wrote:
JN> Ted Zlatanov wrote:
quoted
quoted
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?
See Documentation/SubmittingPatches, section '(5) Sign your work'
for what this means.
If you just forgot to sign off, that's fine and I can forge it or go
without. If you are unable to sign off because you don't have the
right to submit the change under an open source license, I'd be a bit
worried going forward.
[...]
JN> Or more simply, would it make sense to wrap both 'defined' checks into
JN> a single "if", like so?
JN> if (defined $entry->{$check} && defined $query->{$check}) {
JN> ...
JN> } else {
JN> log_debug(...);
JN> }
I prefer the explicit version because we can issue a more precise
log_debug message.
That's fine with me.
After this patch, the code looks like
if (!defined $entry->{$check}) {
log_debug(...);
} elsif (defined $query->{$check}) {
...
} else {
log_debug(...);
}
As a small nit, wouldn't it be more readable with the two !defined()
cases together?
if (!defined $entry->{$check}) {
...
} elsif (!defined $query->{$check}) {
...
} else {
...
}
Thanks again.
Jonathan