[MERGE] deprecate find_previous_heads

Aaron Bentley aaron.bentley at utoronto.ca
Mon Sep 3 17:41:21 BST 2007


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

bb:tweak

Robert Collins wrote:
> +    * Deprecated method ``find_previous_heads`` on
> +      ``bzrlib.inventory.InventoryEntry``. This has been superceded by the use

Actually, "superseded" uses an "s" in the middle.

> +    * New public method ``heads`` on the ``bzrlib.graph.Graph`` class. This is
> +      a simple rename of a previously internal method which implements the same

I think "previously-internal" makes the grouping clearer.

> -    def _filter_candidate_lca(self, candidate_lca):
> -        """Remove candidates which are ancestors of other candidates.
> +    def heads(self, keys):
> +        """Return the heads from amongst keys.

I still find it amusing that I wrote a Graph.heads implementation by
accident.

>          """
> +        candidate_lca = set(keys)

^^^ It's not a blocking issue, but it would be nice to rename
candidate_lca.  It's historical raisins now.

Also, should Graph.heads make any ordering guarantees?  If not, we
should document that.


>         - The kind has not changed (* not sure why this is a clause *)

Well, gannotate says John did it to fix bug 90111.

It makes perfect sense if you're viewing the versionedfile merely as a
content store, because if the leftmost ancestor is a different kind,
your delta compression will be horrible.  But if the rightmost ancestor
is of the same kind, you'll typically get much better delta compression
if it becomes the leftmost ancestor.

I think this is yet another example of the impedence mismatch between
"versionedfiles as content stores" and "versionedfiles as file ancestry".

> +        # --- what follows is now encapsulated in repository.get_graph.heads(), 
> +        #     but that is not accessible from here as we have no repository
> +        #     pointer.

We do have a way to get versionedfiles, and an idea how to convert
versionedfiles into Graphs, but it's probably not worth it for a
deprecated method.

Given that you're introducing a new public method, it would be nice to
test it.  Technically, you're not reducing test coverage, but now a
significant chunk of the testing is testing its use in a deprecated
method.  When that deprecated method goes away, its tests will, too.
And then test coverage will be reduced.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFG3Dkx0F+nu1YWqI0RArXkAJ9VvDA1tEh52wr1r9XaJ7VAaB2WcQCfcfnS
CnKp7Fzj0BNrjELELTy+5V8=
=nskd
-----END PGP SIGNATURE-----



More information about the bazaar mailing list