launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #00612
Re: [Merge] lp:~lifeless/launchpad/registry into lp:launchpad/devel
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.
--
https://code.launchpad.net/~lifeless/launchpad/registry/+merge/32067
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~lifeless/launchpad/registry into lp:launchpad/devel.
References