From: Stefan Beller <sbeller@google.com>
Date: Wed, 11 May 2016 17:41:01 GMT
Subject: Re: [PATCH 3/7] submodule-config: keep labels around
Message-ID: <CAGZ79kYSxccNLvJ-=UghabQ_MG73jRSabVy5rgfNo-A6Fjz_yQ@mail.gmail.com>
In-Reply-To: <xmqqh9e5l7id.fsf@gitster.mtv.corp.google.com>

On Tue, May 10, 2016 at 6:15 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> @@ -199,6 +203,7 @@ static struct submodule *lookup_or_create_by_name(struct submodule_cache *cache,
>>       submodule->update_strategy.command = NULL;
>>       submodule->fetch_recurse = RECURSE_SUBMODULES_NONE;
>>       submodule->ignore = NULL;
>> +     submodule->labels = NULL;
>
> Hmph, is there a reason to do this, instead of embedding an instance
> of "struct string_list" inside submodule structure?
>
> I am not yet claiming that embedding is better.  Just wondering if
> it makes it easier to handle initialization as seen in the hunk
> below, and also _clear() procedure.

Thanks for pointing out that alternative.  That looks so much
better in this patch. Let's see how the follow up patches develop.
As we'd not check != NULL first, but check against the count of the
string list. (I expect no problems down that road though).