Re: [PATCH 1/4] Refactoring: remove duplicated code from transport.c and builtin-send-pack.c
- From
Larry D'Anna <larry@elder-gods.org>
- Date
- Feb 15, 2010, 17:30 UTC
- Message-ID
- <20100215173041.GA8215@cthulhu>
- In-Reply-To
- <7v635zj8jr.fsf@alter.siamese.dyndns.org>
Weird: I only got the Cc for this, git@vger.kernel.org didnt' sent it to me. It doesn't seem to be on gmane either.
* Junio C Hamano (gitster@pobox.com) [100215 01:51]:
Show 28 quoted lines
> Junio C Hamano <gitster@pobox.com> writes: > > > Jeff King <peff@peff.net> writes: > > > >>> builtin-send-pack.c | 89 ++++++++++++++---------- > >>> send-pack.h | 20 +++++ > >>> transport.c | 196 --------------------------------------------------- > >> > >> I think this is backwards. The versions in send-pack were there first, > >> and then were ported to transport.c so that other transports could > >> benefit from them. And that is where they should ultimately be. > > > > Also the names of these functions probably need to be made more specific > > so that people not so familiar with the transport code can tell that they > > are from "transport" family. The names didn't matter much while they were > > file scope static, but this series changes that. > > Ah, one more thing. I think this patch touches somewhat overlapping areas > the ld/push-porcelain topic in 'pu' touches. > > I think Peff's "backwards" observation is correct (and Daniel can > elaborate if he wants). Once the direction is set on that point, you and > Larry probably would need to coordinate to decide how to proceed. My gut > feeling without actually looking at the conflicts is that applying your > code consolidation first and then doing the "porcelain" rework on top > might be a cleaner approach, but you two are in better position to decide > on the order, as these are your codes that will be conflicting with each > other.
That sounds good to me. I'll rebase the porcelain stuff off the next version of Michael's series.
--larry