On Wed, Apr 22, 2009 at 02:24:07PM -0400, Daniel Barkalow wrote:
> On Wed, 22 Apr 2009, Andy Lester wrote:
>
> > There's a ton of code duplicated between transport.c and builtin-send-pack.c,
> > from print_push_status() and its static helpers.
> >
> > Is there a reason NOT to refactor it out of the builtin and use the
> > transport?
>
> I think the builtin should actually just be deprecated and eventually
> removed. As far as I know, nothing actually runs "git send-pack" rather
> than calling send_pack(), but I left the builtin entry point, along with
> its helpers, just in case.
>
> If you're interested in reorganizing things there, I think it would be
> best to move send_pack() to a new send-pack.c, such that
> builtin-send-pack.c can go away entirely.
I think there are actually three issues here:
1. send_pack() is a library-ish function used by transport.c (which in
turn is called by push), and it is in builtin-send-pack.c. This is
generally against git policy.
2. There are several static functions duplicated in transport.c and
builtin-send-pack.c, which can be refactored to exist only once.
In fact, I really don't see why your 64fcef2 didn't do that in the
first place. It looks like they were cut and paste into
transport.c; I don't see why you didn't just make them non-static
and delete the original versions.
3. Nobody really uses "git send-pack" anymore, so it can perhaps be
deprecated and eventually dropped.
I think Andy was referring to (2), and I think that should be cleaned
up, as the different versions have a tendency to diverge. Probably
addressing (1) by moving send_pack() to transport.c makes sense as part
of the same cleanup.
I don't know that (3) really buys us much. Sure, it is probably useless,
but we would need to keep it for historical compatibility for quite some
time, anyway.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html