Re: [PATCH v3 1/5] promisor-remote: refactor to get rid of 'struct strvec'

From: Junio C Hamano <gitster@pobox.com>
Date: May 20, 2025, 16:45 UTC
Message-ID: <xmqqr00ji63l.fsf@gitster.g>
In-Reply-To: <CAP8UFD2XtyPD7b=sE+t9d-kyXDt4hYMqVotwz7hJZEO+5TfXGg@mail.gmail.com>

Christian Couder <christian.couder@gmail.com> writes:

Show 24 quoted lines

> On Tue, May 20, 2025 at 11:37 AM Karthik Nayak <karthik.188@gmail.com> wrote:
>>
>> Christian Couder <christian.couder@gmail.com> 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.