[MERGE] refactor of find_previous_heads from inventory to repository

[John Arbash Meinel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Ian Clatworthy wrote:
> Part of the commit refactoring work driven by the design notes put
> together by Martin and Robert in doc/developers/commit.txt.
> 
> Ian C.
> 

A bit more explanation would have been helpful. Is this only moving code
around, or did you add new code?

You seem to be refactoring a class-level function
(InventoryEntry.find_previous_heads) into a Module level function
(bzrlib.repository.find_previous_heads).

Now, it turns out that you didn't get rid of IE.find_previous_heads, so an
implementation still has the opportunity to override it at that level.

But if you are trying to make it a Repository responsibility, shouldn't it be
Repository.find_previous_heads().

Such that either the Inventory itself knows about the Repository it came from,
or you pass that into the find_previous_heads() function?

Anyway, this seems to actually decrease flexibility, because you take something
that could be implementation defined, and convert it into be fixed.

But as I haven't been privy to the discussion you've had with Martin & Robert I
may just be missing what you are trying to do.


At the very least, I would expect some of the IE.find_previous_heads() tests to
be refactored so that they are testing bzrlib.repository.find_previous_heads()
directly. Rather than assuming it is tested by IE.find_previous_heads(). (If
only because when we decide to get rid of IE.find_previous_heads() someone may
decide that all those tests can just be deleted, since we are removing that
function, not realizing that it was actually testing some other function.)

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGhIRwJdeBCYSNAAMRAiWfAKDEeoWL7M8dc/ggu1PL5+D1GxkBYgCg0CPW
gELJaCoS7VdQHkQ2HUykaXw=
=mWmq
-----END PGP SIGNATURE-----