Code review comment for lp:~wallyworld/launchpad/disabled-project-bugs-shown

> 32 + if len(self.bugtasks):
> 33 + self.milestones = list(
> 34 + bugtask_set.getBugTaskTargetMilestones(self.bugtasks))
>
> Does it break if self.bugtasks is empty? If not, why the guard?
>

Yes. getBugTaskTargetMilestones() breaks.

> 72 - self.assertThat(recorder, HasQueryCount(LessThan(67)))
> 73 + self.assertThat(recorder, HasQueryCount(LessThan(69)))
>
> I only see one new query. What's the second?
>

From what I saw, the section of code with the extra query is actually
called twice when the page loads. I've not yet tracked down why.

> 103 + foo_bug = self.factory.makeBug(product=product_foo)
> 104 + bugtask_set = getUtility(IBugTaskSet)
> 105 + bugtask_set.createTask(
> 106 + bug=foo_bug, owner=foo_bug.owner, product=product_bar)
>
> Consider Bug.addTask or LOF.makeBugTask for the second task.
>

Ok. I copied from what was in the doctest. Bad move perhaps :-)

> 108 + removeSecurityProxy(product_bar).active = False
> 132 + removeSecurityProxy(product_foo).active = False
>
> Consider letting LOF.makeProduct take an "active" parameter.

Considered it but there's already a lot of parameters there and the
number of times we need to change active to false didn't seem to justify
the additional clutter.