Re: [PATCH 1/3] Move bundle specific stuff into bundle.[ch]

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi,

On Tue, 17 Jul 2007, Daniel Barkalow wrote:

> On Tue, 17 Jul 2007, Johannes Schindelin wrote:
> 
> > The transport specific stuff was moved into libgit.a, and the bundle 
> > specific stuff will not be left behind.
> > 
> > This is a big code move, with one exception: the function unbundle() 
> > no longer outputs the list of refs.  You have to call 
> > list_bundle_refs() yourself for that.
> 
> You should use -C on this sort of thing, so that the interesting aspects 
> of the patch are easier to see. (It actually comes out longer in this 
> case, but it's far easier to tell that the code in the new file is the 
> same as the old code.)

Okay, I wanted it to be kept short, since I really get lost easily in 
hundreds of "-" lines, with possibly one in the midst being a "+".

> Can you tell I've been rearranging a lot of code lately and trying to 
> make the patches not look really scary?

Sorry, I did not pay that close attention.

> Aside from presentation, it looks good to me. Shall I stick the bundle 
> changes into my series? I'd like to have them come before the patch to 
> switch to builtin-fetch, so that there aren't any revisions where "git 
> fetch" doesn't have bundle support.

Looks fine to me.  Seems like you should add a SOB line, too.

> And I think it would be best to take part 3 as a review fix to my final 
> patch.

Yes, definitely.  This shows again (to me, at least), that just looking at 
the code is not enough, you have to run it, too, to review patches.

Ciao,
Dscho