From: Pablo Sabater <pabloosabaterr@gmail.com>
Date: Sat, 27 Jun 2026 21:03:09 GMT
Subject: Re: [PATCH GSoC v14 06/13] fetch-pack: move function to connect.c
Message-ID: <CAN5EUNRP7ZBxP-Q_eWrjdh5hY3+WC92nWPhyjYScTdzZWOkZcg@mail.gmail.com>
In-Reply-To: <CAOLa=ZRhCXBQN7CqwLyE2F9u+oqAsqvcFP+fuiyw4SVaSDfT6Q@mail.gmail.com>

El vie, 26 jun 2026 a las 18:57, Karthik Nayak
(<karthik.188@gmail.com>) escribió:
>
> Pablo Sabater <pabloosabaterr@gmail.com> writes:
>
> Nit: could we mention the function in the subject?

Yes, I'll mention it.

>
> > write_fetch_command_and_capabilities will be refactored in a
> > subsequent
>
> Super Nit: Some parts of your patches use backticks for quoting code or
> filenames and others skip the convention. It would be nice to be
> consistent.

Sorry about that, I'll stick to backticks always.

>
> > commit where it will become a more general-purpose function, making it
> > more accessible to additional commands in the future.
> >
> > Move `write_fetch_command_and_capabilities()` to `connect.c`, where
> > there are similar purpose functions.
> >
> > Because string_list is only used as a pointer, use a forward
> > declaration [1].
> >
> > [1]: https://lore.kernel.org/git/Z0RIqUAoEob8lGfM@pks.im/
> >
> > Helped-by: Jonathan Tan <jonathantanmy@google.com>
> > Helped-by: Christian Couder <chriscool@tuxfamily.org>
> > Signed-off-by: Calvin Wan <calvinwan@google.com>
> > Signed-off-by: Eric Ju <eric.peijian@gmail.com>
> > Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
> > ---
> >  connect.c    | 34 ++++++++++++++++++++++++++++++++++
> >  connect.h    |  4 ++++
> >  fetch-pack.c | 34 ----------------------------------
> >  3 files changed, 38 insertions(+), 34 deletions(-)
> >
[snip]

Thanks for the feedback,
Pablo.