From: Jonathan Nieder <jrnieder@gmail.com>
To: Ted Zlatanov <tzz@lifelogs.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] git-credential-netrc: fix uninitialized warning
Date: Tue, 8 Oct 2013 13:02:35 -0700 [thread overview]
Message-ID: <20131008200235.GG9464@google.com> (raw)
In-Reply-To: <87li23v8p5.fsf@flea.lifelogs.com>
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?
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
next prev parent reply other threads:[~2013-10-08 20:02 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-08 14:34 [PATCH] git-credential-netrc: fix uninitialized warning Ted Zlatanov
2013-10-08 19:41 ` Jonathan Nieder
2013-10-08 19:55 ` Ted Zlatanov
2013-10-08 19:58 ` Stefan Beller
2013-10-08 20:04 ` Ted Zlatanov
2013-10-08 20:02 ` Jonathan Nieder [this message]
2013-10-08 20:12 ` Ted Zlatanov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20131008200235.GG9464@google.com \
--to=jrnieder@gmail.com \
--cc=git@vger.kernel.org \
--cc=tzz@lifelogs.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).