Re: [PATCH 08/21] remote: let guess_remote_head() optionally return all matches

From: Jeff King <peff@peff.net>
Date: Feb 27, 2009, 11:43 UTC
Message-ID: <20090227114308.GA20776@coredump.intra.peff.net>
In-Reply-To: <76718490902261047p6168e875wf29bcca780105d3e@mail.gmail.com>

On Thu, Feb 26, 2009 at 01:47:45PM -0500, Jay Soffian wrote:

Show 6 quoted lines

> Well, if you wanted to be consistent about things (and I apologize if gmail
> mangles the lines), I'd probably do something like:
> [...]
> Then peer_ref is consistently a copy, so we can free it consistently, we don't
> need two separate copy functions, and copy_ref returns NULL upon receiving
> NULL like most of the other foo_ref functions.

Good point. That is much cleaner, IMHO, and probably worth doing as part of this series. Though I hesitate to make you reroll _again_ for such a small cleanup. Maybe it is worth just putting on top.

Show 6 quoted lines

> > Rather than having ref structs with "next" and "peer" pointers, I think
> > a more natural data structure would be a list (or array) of "ref pairs".
> 
> Actually, we don't need most of the fields in the peer_ref, so we could
> probably just embed the extra fields that we need in a peer_struct inside the
> ref struct. I can add this to my git todo list.

Sure, that might turn out cleaner (though you may run into problems where you want to pass a "struct ref" to a helper function but you have only the fake "peer_struct").

But while that cleanup might be nice, I don't think it is probably worth the pain, assuming you are done messing with remotes for a little while, now.

-Peff