Christian Couder <christian.couder@xxxxxxxxx> writes: > On Tue, May 20, 2025 at 11:37 AM Karthik Nayak <karthik.188@xxxxxxxxx> wrote: >> >> Christian Couder <christian.couder@xxxxxxxxx> writes: >> >> [snip] >> >> > >> > /* >> > - * Find first index of 'nicks' where there is 'nick'. 'nick' is >> > - * compared case sensitively to the strings in 'nicks'. If not found >> > - * 'nicks->nr' is returned. >> > + * Find first element of 'p' where the 'name' member is 'nick'. 'nick' >> > + * is compared case sensitively to the strings in 'p'. If not found >> > + * NULL is returned. >> > */ >> > -static size_t remote_nick_find(struct strvec *nicks, const char *nick) >> > +static struct promisor_info *remote_nick_find(struct promisor_info *p, const char *nick) >> >> Nit: while we're here wouldn't be nicer to rename this to >> `promiser_info_list_find_name` or similar? > > 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. Is promisor_info primarily be found by their names before its other members are accessed? 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. Thanks.
