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

Revision history for this message
Robert Collins (lifeless) wrote :

This needs an incremental review.

Please ignore the cachedproperty.py changes other than those in rev 11324 - they are in my separate, approved, lp:~lifeless/launchpad/cachedproperty branch - that I started after this review, but merged in because it makes the branch cleaner and leaner.

You could pull that branch and merge this into it to see the incremental changes.

I'm particularly interested in the removeSecurityProxy calls in cachedproperty.py: I think they are appropriate and needed, but perhaps not?

The change in 11323 to invalidate the cache on Person is definitely needed and an expected consequence of more model object caching; taste wise we probably want to iterate towards an interface where the cache description ('_archive_cached') matches the cache attribute ('.archive') - but I think that that is essentially polish, not a prerequisite.

« Back to merge proposal