From: Junio C Hamano <gitster@pobox.com>
Date: Fri, 07 Oct 2022 18:35:45 GMT
Subject: Re: [PATCH 2/2] removed else statement
Message-ID: <xmqqpmf3eb26.fsf@gitster.g>
In-Reply-To: <44d2f602-a299-3bd0-1624-cfdddb6dcda7@github.com>

Victoria Dye <vdye@github.com> writes:

> dsal3389 via GitGitGadget wrote:
>> From: dsal3389 <dsal3389@gmail.com>
>> 
>> there is no need for the else statement if we can do it more
>> elegantly with a signle if statement we no "else"
>
> Similar recommendations on the commit message as in the previous patch [1]:
>
> - title should be prefixed with 'git.c:'
> - title & message should use the imperative mood (e.g. "remove else
>   statement" instead of "removed else statement")
> - please fix typos
>     - s/there/There
>     - s/signle/single
>     - s/we/with(?)

Thanks for these.

>> +	if (argc <= 0) {
>
> nit: argc is always >= 0 [2], so a more appropriate condition would be:
>
>     if (!argc)
>
> There are lots of examples of that '!argc' conditional in Git, but none of
> the 'argc <= 0' pattern, so it's probably best to match convention here.

Yes to this, too.