Code review comment for lp:~lifeless/launchpad/registry

Revision history for this message
Henning Eggers (henninge) wrote :

Hi Robert,
thanks for all this work. We talked about the use of removeSecurityProxy in cachedproperty.py. It has to be clear that these are low-level functions that don't care about security. Security has to be considered as each call site. So this is mainly a documentation issue.

Please use "naked_instance" in the functions, though, to be clear about it.

Also, we agreed that you file a tech-debt bug about moving the code to lp.services and creating an extra doctest file instead of tests in docstring. Adding a unittest for extra points. ;-)

We will have to watch how these functions will be used in the future.

Cheers,
Henning

review: Approve (code)

« Back to merge proposal