Re: [PATCH 3/3] string_list: Remove string_list_insert_at_index from its API

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Nov 24, 2014 at 4:25 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Stefan Beller <sbeller@xxxxxxxxxx> 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?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]