launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

Review: Approve code

137	+ grantee_ids = [grantee[0].id for grantee in grant_permissions]

This is not an entirely insignificant change: it causes grant_permissions to be iterated before the main loop. It looks like all callsites currently pass in a list, but if it happens to be a ResultSet then the pre-consumption will cause jsonShareeData to return an empty list. Hopefully not a concern here, though.

148	+ image_url = display_api.custom_icon_url() or ''

I think it feels slightly nicer to use None instead of ''. I'd also consider s/image/icon/.

149	+ sprite_css = display_api.sprite_css() or 'sprite bullet'

Can sprite_css for a person or team ever return something that evaluates to false? This seems like over-nice handling of a case that should at best crash.
-- 
https://code.launchpad.net/~wallyworld/launchpad/sharee-display-details-1002954/+merge/107912
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.