From: Eric Sunshine <sunshine@sunshineco.com>
Date: Fri, 30 Oct 2015 18:16:19 GMT
Subject: Re: [PATCHv2 2/8] submodule config: keep update strategy around
Message-ID: <CAPig+cRh9J0izFvLzRjjU4FEBKJsiJaYFv=9WdxFVfJ3xs0JiQ@mail.gmail.com>
In-Reply-To: <CAGZ79kZ1usWVutWwyFQKeujyyTPVRtSQM6dvkU9gWUDSTNpB6w@mail.gmail.com>

On Fri, Oct 30, 2015 at 1:38 PM, Stefan Beller <sbeller@google.com> wrote:
> On Thu, Oct 29, 2015 at 6:14 PM, Eric Sunshine <ericsunshine@gmail.com> wrote:
>>> +               else if (!me->overwrite && submodule->update != NULL)
>>
>> Although "foo != NULL" is unusual in this code-base, it is used
>> elsewhere in this file, including just outside the context seen above.
>> Okay.
>
> ok, I'll clean that up as we go.

Oh, I wasn't suggesting that you clean this up (though you may if you
want). I was merely commenting (for the sake of others reviewing this
patch) that, while not the norm for the project, this instance is
consistent with surrounding code.

>>> +                       free((void *)submodule->update);
>>
>> Minor: Every other 'free((void *) foo)' in this file has a space after
>> "(void *)", one of which can be seen in the context just above.
>
> done