On Tue, May 20, 2025 at 6:45 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Christian Couder <christian.couder@xxxxxxxxx> writes: > > Junio suggested this name in a discussion of a previous patch series: > > > > https://lore.kernel.org/git/xmqqa5bbq0nb.fsf@gitster.g/ > > Don't blame me for that name ;-) > > The name was for a hypothetical variant that took "struct strvec *" > as its first parameter, and the name was given only because it did > not make much sense to call the helper after "strvec". A function > name that signals that we are finding (something) using "nick"-name > was a much better choice. Since your final one finds in "struct > promisor_info *", not a generic "struct strvec *", I wouldn't be > surprised if a name that is about promisor-info (whatever it is) > more clearly describes what it does. If this is a file-local > helper, promisor_info_find() or find_promisor_info() may be > sufficient, if "by nickname" is the primary and only way for the > application to find an instance of promisor_info. OK, I think I will use promisor_info_find() or promisor_info_find_by_name() in the next reroll, as it goes well with how other functions related to "struct promisor_info *" are named. > Is promisor_info primarily be found by their names before its other > members are accessed? No, there are different ways the promisor_info instances are iterated on or searched, see promisor_remote_info() and all_fields_match() where we iterate on all the instances and then either use the instance members to generate an advertisement string, or check if some of the members match advertised values. > If so, I wonder if a strmap or string_list > that uses the nickname as the key with a pointer to a promisor_info > structure as the data more appropriate than a hand-crafted linear > linked list. I don't think it would bring a lot of benefits. Using an strmap or a sorted string list might make things faster if there are a lot of promisor remotes configured on the clients, but I don't think we are at a point where such an optimisation is worth it.
