> Just a few comments below, mostly simple clarity stuff.  Thanks for
> fixing the icky bug!

Yeah, it'll be good to get this cleaned up.

> > +    def branchIdentites():
> Missing an 'i' here :)

Fixed.
 
> > +        """A list of aliases for a branch.
> > +
> > +        Returns a list of tuples of bzr identity and context object. 
> > There is +        at least one alias for any branch, and that is the
> > branch itself.  For +        linked branches, the context object is the
> > appropriate linked object. +        """
> 
> I'm afraid these two docstrings both need work.  The first doesn't
> explain which ICanHasLinkedBranch objects are returned, nor what being
> 'sorted' means in this context.
> 
> The second I think needs to explain itself a bit more as well.  It
> seems to blur the concept of 'alias' (a concept we don't have anywhere
> else) and the linked object that the alias is derived from.  Can you
> just try to expand it a bit, maybe with an example or two?
> 
> In this branch it's not really obvious why branchLinks and
> branchIdentities have a separate existence to the bzr_identity
> property.  I guess this is preparation for that portlet you want to
> add?  I hope if the implementation of that doesn't require all these
> variants we can shrink them down a bit.

Updated.

> > === modified file 'lib/lp/code/model/linkedbranch.py'
> > +            # When a project gets the series they are ordered
> > alphabetically +            # by name.
> 
> I don't think this sentence makes sense.
> 
> I don't suppose it really matters, but sorting series by name isn't
> how it's usually done in Launchpad; sorting trunk first and then by
> comparing date_created is more common I think?

I looked at the product code to see how it returned them. Generally it just 
sorted alphabetically, which I think is OK.

Comment updated too.

> > === modified file 'lib/lp/code/model/tests/test_branch.py'
> > +class TestBranchLinksAndIdentites(TestCaseWithFactory):
> > +    """Test IBranch.branchLinks and IBranch.branchIdentities."""
> > +
> > +    layer = DatabaseFunctionalLayer
> > +
> > +    def test_default_identities(self):
> > +        # If there are no links, the branch identities is just the
> > unique +        # name.
> 
> This sentence isn't very grammatical, maybe "If there are no links,
> the only branch identity is the unique name"?

Updated.
 
> > +    def test_linked_to_product_series(self):
> > +        # If a branch is linked to a non-development series of a product
> > and +        # not linked to the product itself, then the product is not
> > returned +        # in the links.
> 
> I think the more positive "only the product series is returned in the
> links" is easier to understand than "the product is not returned".

Updated too.

> > +    def test_linked_to_package_not_current_series(self):
> > +        # If a branch is the development focus branch for a product,
> > then it's +        # bzr identity is lp:product.
> 
> This comment looks entirely wrong :-)

And this.
 
> >  class TestBzrIdentity(TestCaseWithFactory):
> >      """Test IBranch.bzr_identity."""
> 
> You don't test here any branches that are linked to both products and
> source packages, say.  Is that intentional?  I realise the
> TestLinkedBranchSorting tests basically cover this ground.

Yes it was intentional.  Given that we test that both types of url shortening 
happen, and we test the sorting elsewhere, I didn't think it was necessary.
 
> > === modified file 'lib/lp/code/model/tests/test_linkedbranch.py'
> > +    def test_product_sort(self):
> > +        # If in the extremely unlikely event we have one branch linked
> > as the +        # trunk of two different products (you never know), then
> > the sorting
> 
> Given the test, I think you should say "two or more" here.

Updated.