On Tue, 11 Sep 2007, Junio C Hamano wrote:
> 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.
> 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.
> 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.
> 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.
> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
