Code review comment for lp:~al-maisan/launchpad/flip-the-switch-516545

On 02/10/2010 11:51 AM, Gavin Panella wrote:
> Review: Approve
> Hi Muharem,
>
> All looks good. One tiny comment in below.
Hello Gavin,

your point below is well made. Thanks!

>> === modified file 'lib/lp/soyuz/doc/build.txt'
>> --- lib/lp/soyuz/doc/build.txt 2010-01-21 08:53:21 +0000
>> +++ lib/lp/soyuz/doc/build.txt 2010-02-10 10:33:18 +0000
>> @@ -8,6 +8,7 @@
>> launchpad build daemon.
>>
>> # Create a 'mozilla-firefox' build in ubuntutest/breezy-autotest/i386.
>> + >>> from zope.security.proxy import removeSecurityProxy
>> >>> from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
>> >>> login('<email address hidden>')
>> >>> test_publisher = SoyuzTestPublisher()
>> @@ -17,6 +18,9 @@
>> >>> binaries = test_publisher.getPubBinaries(
>> ... binaryname='firefox', pub_source=source)
>> >>> [firefox_build] = source.getBuilds()
>> + >>> the_job = removeSecurityProxy(firefox_build.buildqueue_record.job)
>> + >>> the_job.start()
>> + >>> the_job.complete()
>
> I think it's bad to keep a reference to an unwrapped object that is
> normally subject to access control. It's too easy to inadvertently
> reuse this reference later in the test. Would you be happy to do
> something like:
>
> job = firefox_build.buildqueue_record.job
> removeSecurityProxy(job).start()
> removeSecurityProxy(job).complete()
>
> instead?
That's much better indeed. Done. Thanks again!

Best regards

--
Muharem Hrnjadovic <email address hidden>
Public key id : B2BBFCFC
Key fingerprint : A5A3 CC67 2B87 D641 103F 5602 219F 6B60 B2BB FCFC