[MERGE REVIEW] Move BzrBranch.get_revision_delta to Branch

[Aaron Bentley]

Jelmer Vernooij wrote:
> Any chance somebody could have a look at this patch and approve it or
> comment? 

I don't like this method much, as I prefer the minimal-but-complete
approach to object interfaces, and Branch definitely does not have that.
    It has only one caller in the bzr codebase, so I'd welcome
deprecating it, though others would doubtless disagree.

Still, I'd hate for there to be multiple implementations of it.  Your
port of the method from BzrBranch to Branch looks fine. +1 from me.

On the other hand, there are some issues with the method itself.  Fixing
those issues would be nice, but not required for a merge.

> === modified file 'a/bzrlib/branch.py'
> --- a/bzrlib/branch.py	
> +++ b/bzrlib/branch.py	
> @@ -517,6 +517,26 @@
>          if parent:
>              destination.set_parent(parent)
>  
> +    def get_revision_delta(self, revno):
> +        """Return the delta for one revision.
> +
> +        The delta is relative to its mainline predecessor, or the
> +        empty tree for revision 1.
> +        """

The use of 'mainline' is confusing here.  Mainline usually refers to the
main branch of a project, not to the revision-history of an individual
branch. I think 'revision-history predecessor' would be more appropriate.

> +        assert isinstance(revno, int)
> +        rh = self.revision_history()
> +        if not (1 <= revno <= len(rh)):
> +            raise InvalidRevisionNumber(revno)
> +
> +        # revno is 1-based; list is 0-based
> +
> +        new_tree = self.repository.revision_tree(rh[revno-1])

This looks redundant with get_rev_id.

Aaron