Re: [PATCH 1/6] fetch-pack: refactor packet writing

>
> Peijian Ju <eric.peijian@gmail.com> writes:
> [snip]
>
> >> Right, this commit in itself looks good. But I was curious why we need
> >> this, so I did a sneak peak into the following commits.
> >>
> >> To summarize, we want to call:
> >>    `write_command_and_capabilities(..., "object-info");`
> >> in the upcoming patches to get the object-info details from the server.
> >> But isn't this function too specific to the "fetch" command to be
> >> generalized to be for "object-info" too?
> >>
> >> Wouldn't it make sense to add a custom function for 'object-info' in
> >> 'connect.c'? Like how we currently have `get_remote_bundle_uri()` for
> >> 'bundle-uri' and `get_remote_refs` for 'ls-refs'?
> >
> > Thank you. I am reading through the old comments left by Taylor
> > at https://lore.kernel.org/git/YkOPyc9tUfe2Tozx@nand.local/
> >
> >   " Makes obvious sense, and this was something that jumped out to me when I
> >   looked at the first and second versions of this patch. I'm glad that
> >   this is getting factored out."
> >
> >
> > It seems refactoring this into a more general function is on purpose.
> > It is encouraged to use this general function to request capability
> > rather than adding a custom function.
> > Taylor’s comment was 2 years ago, but I think refactoring this into a
> > more general function to
> > enforce DRY still makes sense.
>
> It would make sense then to move the existing users to also use
> `write_command_and_capabilities` eventually. I guess this could be done
> in a follow up series.
>
> Then I would say `write_command_and_capabilities()` should be moved to
> `transport.c`, no?