Code review comment for lp:~jamalta/launchpad/bug-127489

Hi Jamal,

Thank you for working on this bug!

As Stuart commented, this will need a pagetest. In fact, there is
already one that is probably broken by this change. Have a look at the
end of lib/lp/registry/stories/foaf/xx-person-home.txt.

Did you have a mentor for this branch, or a pre-implementation
discussion? Perhaps that might have spotted the need to update the
pagetest. Have a look at https://dev.launchpad.net/PatchSubmission if
you haven't already. It's the same process as all the Launchpad devs
from Canonical use.

I have a comment about the code, because the cmp argument to sorted()
is deprecated (and I think it's gone entirely in Python 3).

Thanks again!

Gavin.

> === modified file 'lib/lp/registry/browser/person.py'
> --- lib/lp/registry/browser/person.py 2009-07-29 15:34:46 +0000
> +++ lib/lp/registry/browser/person.py 2009-07-31 21:12:09 +0000
> @@ -2517,7 +2517,10 @@
> categories = set()
> for contrib in self.contributions:
> categories.update(category for category in contrib['categories'])
> - return sorted(categories, key=attrgetter('title'))
> + sort = {'code': 0, 'bugs': 1, 'blueprints': 2, 'translations': 3,
> + 'answers': 4, 'specs': 5, 'soyuz': 6}
> + return sorted(categories, key=attrgetter('name'),
> + cmp=lambda x,y: cmp(sort[x], sort[y]))

Because cmp is deprecated, I think this would be better as:

       return sorted(categories, key=lambda category: sort[category.name])

>
> @cachedproperty
> def context_is_probably_a_team(self):