[MERGE] Factor out the Graph.heads() cache from _RevisionTextVersionCache for reuse, and use it in commit.

[Robert Collins]

On Thu, 2007-10-18 at 15:49 +1000, Andrew Bennetts wrote:
> Robert Collins wrote:
> > On Wed, 2007-10-17 at 21:51 -0500, John Arbash Meinel wrote:
> [...]
> > > I would argue that you let them create their own sets. Rather than forcibly
> > > creating a new object every time you enter this function.
> > > 
> > > If *all* callers need to modify the set, then fine. But I doubt that is the case.
> > > 
> > > Maybe it is an API break... but probably a rather minor one at that.
> > 
> > Well the cache is new, but it seems like a good idea to me to have its
> > api match that of the function it wraps, so that the cache can be
> > inserted and removed seamlessly.
> 
> I think it's good to have an API that's safe by default (and just like the API
> it is a cache for).  If we make it easy to corrupt the cache we're inviting
> confusing bugs down the track.  
> 
> So, I think returning a copy is fine, just add a brief comment in the code to
> make it clear that making a copy is the intent, so that future reviewers don't
> look at the code and think “huh, that's odd”.
> 
> Caching and returning a frozenset would be just as fine from a safety point of
> view, so I'd say Robert should just do whichever way is more convenient for him.

I'd prefer to keep the two API's completely in sync. I'll document the
reason that it returns a set in the heads() method on HeadsCache.

-Rob

-- 
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20071018/eec065ab/attachment.pgp