Code review comment for lp:~lifeless/launchpad/bugtask+index

Revision history for this message
Jonathan Lange (jml) wrote :

Hey Rob,

Looks good to me. Very pleased with the improvements to performance & API clarity.

Only a few minor code clarity & style issues. Please fix them & land.

jml

In def _bug_attachments(self):
  * Spaces after colons
  * Proper sentence case for the docstring.

The comment:
+ # Unwrap the security proxy.
confuses me. I don't see how it's unwrapping the security proxy, or why you would need to. Could you please either delete the comment or expand it?

The "sort and group" comments don't help. Please delete them.

"# Note that this query and the tp query above can be consolidated" in model/person.py has a spelling mistake. ITYM "top query".

review: Approve

« Back to merge proposal