Code review comment for lp:~james-w/launchpad/no-more-sampledata-2

Revision history for this message
Francis J. Lacoste (flacoste) wrote :

On August 6, 2010, Abel Deuring wrote:
> Review: Approve code
> Hi James,
>
> again, a nice branch!
>
> Just one minor remark:
> > @@ -540,7 +518,8 @@
> >
> > for pub in pubs:
> > self.checkPastDate(pub.datesuperseded)
> >
> > if supersededby is not None:
> > - if isinstance(pub, BinaryPackagePublishingHistory):
> > + if isinstance(
> >
> > + removeSecurityProxy(pub),
BinaryPackagePublishingHistory):
> > dominant = supersededby.binarypackagerelease.build
> >
> > else:
> > dominant = supersededby.sourcepackagerelease
>
> I think this is one occasion where you don't need to call
> removeSecurityProxy(): The Zope security machinery monkeypatches
> isinstance() so that a possibly existing security proxy is removed before
> the "real" isinstance function is called.

It doesn't monkeypatch by default, but it is common to shadow it by importing

from zope.security.proxy import isinstance

And that one will do the right thing.

--
Francis J. Lacoste
<email address hidden>

« Back to merge proposal