Re: [PATCH 2/4] submodule-config: keep groups around

From: Stefan Beller <sbeller@google.com>
Date: Jan 21, 2016, 00:20 UTC
Message-ID: <CAGZ79kZXSUkMx=+P0+L4yastYzqeCAUf83L2t8H9=QAC1FwuiA@mail.gmail.com>
In-Reply-To: <xmqqwpr43pyz.fsf@gitster.mtv.corp.google.com>

On Wed, Jan 20, 2016 at 1:23 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 6 quoted lines

>>       free((void *) entry->config->name);
>> +     if (entry->config->groups)
>> +             string_list_clear(entry->config->groups, 0);
>
> You are allocating entry->config->groups itself in the in the hunk
> around ll.327, so that also need to be freed, I would think.

done

Show 10 quoted lines

>>       unsigned char gitmodules_sha1[20];
>> +     struct string_list *groups;
>>  };
>
> Is there a case where you need to enumerate and show the groups a
> submodule belongs to to the end users?  Using string_list_insert()
> to manage this list would mean you will lose the original ordering
> you saw the list of groups in their .gitmodules files, which may or
> may not matter.  I'd assume that the ordering should not matter, but
> that is something the user may want to see documented.

In this patch series we only need to check the items of that list one by one, no need for a sorted version, so I'll change it to be unsorted and document that.

>
>>
>>  int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg);