Code review comment for lp:~al-maisan/launchpad/spec-job-class-503839

Michael Nelson wrote:
> On Thu, Jan 7, 2010 at 8:57 PM, Muharem Hrnjadovic
> <email address hidden> wrote:
>> Michael Nelson wrote:
>> [..]
>>
>> Hello Michael,
>>
>> thanks again for taking the time to review this branch despite your own
>> busy schedule. That's very much appreciated :)
>
> No probs!
>
>>> Some comments in line below.
>> Please see my replies as well as the enclosed incremental diff.
>>
>>>> === modified file 'lib/lp/soyuz/model/buildqueue.py'
>>>> --- lib/lp/soyuz/model/buildqueue.py 2010-01-04 14:30:52 +0000
>>>> +++ lib/lp/soyuz/model/buildqueue.py 2010-01-07 08:23:28 +0000
>>>> @@ -57,11 +57,31 @@
>>>> return IBuildFarmJobBehavior(self.specific_job)
>>>>
>>>> @property
>>>> + def specific_job_classes(self):
>>>> + """See `IBuildQueue`."""
>>>> + from zope.component import getSiteManager
>>>> + from lp.buildmaster.interfaces.buildfarmjob import IBuildFarmJob
>>> Is there a reason for these imports to be inline? If so, please add a
>>> comment or otherwise move them as per the guidelines.
>> No other code in the module uses them. I have added a comment to that
>> effect.
>
> Erm... I didn't realise that was a valid reason? I thought we only did
> so if we *had* to (ie. circular imports etc.). Please check our
> styleguide... I'm going from memory but I thought the imports should
> always be at the top even if no other code in the module uses them.

Whoops .. sorry .. and thanks for catching this :)

> Cheers, and happy flying!

Thank you! Have a safe journey as well :)

Best regards

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