On Tue, 11 Sep 2007, Junio C Hamano wrote:

quoted text
> Very impressed by the entire series and mildly amused.  Thanks.

You're welcome. I should mention that I've sent all of this in before, 
although not in a single series; the verdict was "post-1.5.3", so I tucked 
it away and just now dug it out and rebased it.

quoted text
> A few C++ style comments sprinkled here and there were eyesore,
> but I'll hopefully survive.

Go ahead and fix them. It's not a coding style thing that I really notice 
one way or the other, so I don't mind other people changing them but I 
tend not to conform myself all that reliably.

quoted text
> You seem to have a stray printf("connect to ...\n"); as the
> command is expected to show the list of refs it fetched from the
> other side, this may interfere with what the caller wants to do.

I realized after using this for a while that I have no idea exactly what 
"git fetch" is expected to print. I should be able to match any particular 
style, but I can't really follow the shell version, and Julian didn't 
replicate the output in his original C version.

quoted text
> The changes to t5515 test vectors worry me quite a lot, as the
> distinction between not-for-merge and others is what decides the
> outcome of a pull, but I didn't look very closely.

These changes are described in the corresponding commit message. They're 
all for config files where nobody want actually want to do it like that. 

The more likely one to come up is when the "merge" option and the fetch 
lhs both specify the same ref, but they use different abbreviations. My 
code expands both of the abbreviations (which are unambiguous anyway) 
before comparing them; the old code compared them first.

The less likely one is that, if you use the ancient "branches" config 
files, and also use the new config file, you can specify a non-default 
"merge" option. Nothing good can come of this, because the "branches" 
config only configures one remote ref. The shell version of fetch ignored 
the "merge" option; mine obeys it.

quoted text
> Unfortunately I do not have enough time for a full review during
> the workday evenings.  Hopefully will take a deper look on my
> next git day.

Sure. I've been working on other stuff entirely for a while; just wanted 
to get this out now that 1.5.3 is done. (And with the GSoC conversions, I 
think we have all of the major core operations.)

I've also started on more stuff for after this, converting things like 
ls-remote, and also reducing the number of ssh connections that fetch does 
by one, but those aren't done and I haven't been thinking about then 
recently, anyway.

	-Daniel
*This .sig left intentionally blank*
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html