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

From: Christian Couder <christian.couder@gmail.com>
Date: May 20, 2025, 13:32 UTC
Message-ID: <CAP8UFD2XtyPD7b=sE+t9d-kyXDt4hYMqVotwz7hJZEO+5TfXGg@mail.gmail.com>
In-Reply-To: <CAOLa=ZRnL_PLVpQrmHPMXtJd9e965B5iz8widkk5GNSDvh21Pg@mail.gmail.com>

On Tue, May 20, 2025 at 11:37 AM Karthik Nayak <karthik.188@gmail.com> wrote:

Show 19 quoted lines

>
> 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/

I am not sure that changing its first argument from a `struct strvec *` to a `struct promisor_info *` makes it worth renaming. If that's the case, then I would be fine with renaming it like you suggest though.