From: Jeff King <peff@peff.net>
Date: Fri, 27 Feb 2009 11:43:08 GMT
Subject: Re: [PATCH 08/21] remote: let guess_remote_head() optionally return all matches
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:

> 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.

> > 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