Re: [PATCH] git-credential-netrc: fix uninitialized warning
On Tue, 8 Oct 2013 13:02:35 -0700 Jonathan Nieder wrote:
JN> Ted Zlatanov wrote:
>> On Tue, 8 Oct 2013 12:41:47 -0700 Jonathan Nieder
>> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-credential-netrc: fix uninitialized warning
On Tue, 08 Oct 2013 21:58:41 +0200 Stefan Beller wrote: SB> On 10/08/2013 09:55 PM, Ted Zlatanov wrote: JN> Sign-off? >> >> I didn't realize it was a requirement, must I? SB> Yes, this is a requirement. See Documentation/SubmittingPatches SB> to read what signing off actually means here. OK, I didn't realize it was. Thanks for explaining. Ted -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-credential-netrc: fix uninitialized warning
Ted Zlatanov wrote:
> On Tue, 8 Oct 2013 12:41:47 -0700 Jonathan Nieder 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-credential-netrc: fix uninitialized warning
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 signature.asc Description: OpenPGP digital signature
Re: [PATCH] git-credential-netrc: fix uninitialized warning
On Tue, 8 Oct 2013 12:41:47 -0700 Jonathan Nieder 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> [...]
>> --- 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-credential-netrc: fix uninitialized warning
Hi,
Ted Zlatanov wrote:
> Simple patch to avoid unitialized warning and log what we'll do.
Sign-off?
[...]
> --- 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}) {
Style: elsewhere this file seems to use cuddled elses:
} elsif (...) {
Or more simply, would it make sense to wrap both 'defined' checks into
a single "if", like so?
if (defined $entry->{$check} && defined $query->{$check}) {
...
} else {
log_debug(...);
}
Thanks and hope that helps,
Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-credential-netrc: fix uninitialized warning
Simple patch to avoid unitialized warning and log what we'll do.
---
contrib/credential/netrc/git-credential-netrc | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/contrib/credential/netrc/git-credential-netrc
b/contrib/credential/netrc/git-credential-netrc
index 6c51c43..13e537b 100755
--- 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}) {
log_debug("compare %s [%s] to [%s] (entry: %s)",
$check,
$entry->{$check},
--
1.8.1.5
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
