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

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.

Cheers, and happy flying!
-Michael