Re: [PATCH v9 2/6] submodule: rename strbuf variable

>
> Calvin Wan <calvinwan@google.com> writes:
>
> > On Thu, Mar 2, 2023 at 4:25 PM Junio C Hamano <gitster@pobox.com> wrote:
> >>
> >> Calvin Wan <calvinwan@google.com> writes:
> >>
> >> > A prepatory change for a future patch that moves the status parsing
> >> > logic to a separate function.
> >> >
> >> > Signed-off-by: Calvin Wan <calvinwan@google.com>
> >> > ---
> >> >  submodule.c | 23 +++++++++++++----------
> >> >  1 file changed, 13 insertions(+), 10 deletions(-)
> >>
> >> > Subject: Re: [PATCH v9 2/6] submodule: rename strbuf variable
> >>
> >> What strbuf variable renamed to what?
> >>
> >> I have a feeling that squashing this and 3/6 into a single patch,
> >> and pass buf.buf and buf.len to the new helper function without
> >> introducing an intermediate variables in the caller, would make the
> >> resulting code easier to follow.
> >>
> >> In any case, nice factoring out of a useful helper function.
> >>
> >
> > A much earlier version squashed those changes together, but it was
> > recommended to split those changes up; I think I am indifferent either way
> > since the refactoring is clear to me whether it is split up or not.
> > https://lore.kernel.org/git/221012.868rllo545.gmgdl@evledraar.gmail.com/
>
> I am indifferent, either, but with or without them squashed into a
> single patch, "rename strbuf" would not be how you would describe
> the value of this refactoring, which is to make the interface not
> depend on strbuf.  Some callers may have separate <ptr,len> pair
> that is not in strbuf, and with the current interface they are
> forced to wrap the pair in a throw-away strbuf which is not nice.