[MERGE][bug #174625] 'bzr missing' should use Graph ops

[John Arbash Meinel]

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

Ian Clatworthy wrote:
| John Arbash Meinel wrote:
|> The attached patch fixes up 'bzr missing' to use the new graph functions
|> find_unique_ancestors and find_difference. On this machine I see a fairly
|> noticeable improvement for 'bzr missing' of this branch versus bzr.dev.
|
| Sweet. Some minor tweaks suggested below, mainly doc.
|
| bb:tweak
|
|>    BUGFIXES:
|>
|> +    * ``bzr missing`` uses the new ``Graph.find_unique_ancestors`` and
|> +      ``Graph.find_differences`` to determine missing revisions without having
|> +      to search the whole ancestry. (John Arbash Meinel, #174625)
|
| I think this ought to listed under IMPROVEMENTS, not BUGFIXES.
|

I thought about that, but it does have a bug associated with it. And I tend to
put those under BUGFIXES. I'm willing to move it, though.

|> +def find_unmerged(local_branch, remote_branch, restrict='all'):
|>      local_branch.lock_read()
|
| I'd like you to move the _find_unmerged docstring to this function
| as this is the public API. The docstring for _find_unmerged can then
| just refer to it and say "Both branches should already be blocked."

sure, I was originally going to deprecate 'find_unmerged' before I realized that
its api could be easily extended.

|
|> +    :param tip: The tip of mailine
|> +    :return: [(revno, revision_id)] for all revisions in ancestry that
|> +        left-hand parents from tip
|
| s/mailine/mainline/.
| The return clause needs to add " or None if ancestry is None." Also
| add "are" before left-hand parents.
|
|> +    graph = local_branch.repository.get_graph(
|> +                remote_branch.repository)
|> +    local_revno, local_revision_id = local_branch.last_revision_info()
|> +    remote_revno, remote_revision_id = remote_branch.last_revision_info()
|> +    if local_revno == remote_revno and local_revision_id == remote_revision_id:
|> +        # A simple shortcut when the tips are at the same point
|> +        return [], []
|
| I think you ought to only set graph after you've tested the shortcut
| case.
|
| Ian C.
|

Will do.

ps> you forgot to reply to the list

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

iEYEARECAAYFAkgyN9gACgkQJdeBCYSNAAO5GgCfexvs7dTZcGNwCMxV08epGTPb
XlMAn1pvHp+0t+XQhjdpGGGmCWojC7W3
=iF8b
-----END PGP SIGNATURE-----