From: Stefan Beller Date: Wed, 11 May 2016 21:39:57 GMT Subject: Re: [PATCH 3/7] submodule-config: keep labels around Message-ID: In-Reply-To: +Heiko On Wed, May 11, 2016 at 2:28 PM, Junio C Hamano wrote: > Stefan Beller writes: > >> On Tue, May 10, 2016 at 6:15 PM, Junio C Hamano wrote: >>> Stefan Beller 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). > > I also wonder if we can say the same for the .ignore field, by the > way, if we agree that it is a better direction to go. I don't think this is a good idea, as it's just a string, like .{url, name, path} Instead of storing the string we could store an enum {UNTRACKED, DIRTY, ALL, NONE, NOT_INIT} though. That would be better I guess.