Re: [PATCH 3/9] submodule.c: sort changed_submodule_names before searching it

From: Jonathan Tan <jonathantanmy@google.com>
Date: Oct 17, 2018, 21:21 UTC
Message-ID: <20181017212133.195680-1-jonathantanmy@google.com>
In-Reply-To: <20181016181327.107186-4-sbeller@google.com>

> We can string_list_insert() to maintain sorted-ness of the
> list as we find new items, or we can string_list_append() to
> build an unsorted list and sort it at the end just once.

This confused me at first, because neither function is mentioned in the patch.

> As we do not rely on the sortedness while building the
> list, we pick the "append and sort at the end" as it
> has better worst case execution times.

It took me some time to see that you were rejecting the two solutions you listed in the first paragraph, and are instead using a third (that you describe in this paragraph).

The code itself looks fine.

In the future, I think that it's better if this type of patch went into its own patch set - this seems independent of the concerns of this patch set, so splitting up keeps patch sets small.