Re: [PATCH 3/3] string_list: Remove string_list_insert_at_index from its API
- From
Stefan Beller <sbeller@google.com>
- Date
- Nov 25, 2014, 01:16 UTC
- Message-ID
- <CAGZ79kb-QhbQksaOR4+fwyyzkbWege86nqH63T28RH2UKeN7Mg@mail.gmail.com>
- In-Reply-To
- <xmqq7fykunvs.fsf@gitster.dls.corp.google.com>
On Mon, Nov 24, 2014 at 4:25 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 24 quoted lines
> Stefan Beller <sbeller@google.com> writes: > >> This function behaves the same as string_list_insert, just the >> starting indexes for searching, where to insert into the list is also >> a parameter. So if you have knowledge on where to search for the string >> to be inserted, you may have a speed up version of string_list_insert. > > The last half-sentence I am having trouble parsing. If you have > that knowledge where the item should go, you can insert it at the > right location faster than calling string_list_insert()? > > That sounds like a useful feature to me. > > But if nobody uses that feature, there is no point keeping it, no > matter how useful it may sound. So,... I am not sure. > > The function would be error prone if used on a string-list that is > accessed via string_list_insert() API, which makes it only useful if > the user of the API is in full control of the sort order, so I do > not mind removing it and won't be saying "that is useful, we should > keep it and use it more." > > But your log message confuses me.... >
Well, I'm sorry for the confusion, here is what I was trying to say:
We do not remove functionality from the API, but rather remove a a highly optimized version of a function, which we are not using after patch 2 of the series anyway.
We should keep it around only iff we strongly believe there will be a use case later again. But for now I do have the opinion, we're better off removing the function as a smaller API / header file serves us better. Specially new comers may appreciate a smaller API but documented there.
Maybe we can drop the commit message altogether if my thoughts are still confusing?