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

Hi Gavin,

Thanks for your input regarding this bug. I have actually been working on a test for this. I did not have a mentor when I started working on this issue, but went on #launchpad-dev to get some help. Salgado had recommended that I write a unittest to call the view, and make sure the data was returned in the right order, using a LaunchpadObjectFactory. I had been struggling trying to figure out how to do that exactly, and haven't gotten back to it in a few days now. I will take a look at the test you mentioned and make sure I fix it. Also, I appreciate your comment regarding the depracated cmp() function. I am fairly new to Python and was not aware of this. I will fix that shortly as well, using your recommendation.

> 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):