Code review comment for lp:~thumper/launchpad/bmp-notification-recipients

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

Tim Penhey wrote:
> On Fri, 25 Sep 2009 13:52:24 Michael Hudson wrote:
>> Review: Approve
>> Looks fine.
>>
>> The "potato programming" aspect upsets me slightly -- surely you can get
>> all subscribers that can see the merge proposal in one database query! --
>> but at least it's not in the webapp and it's a very important bug to fix,
>> so let's land it.
>
> "potato programming"? Can you explain a bit more?

http://www.divmod.org/trac/wiki/PotatoProgramming

The usual kind of programming using an ORM that results in pages doing
500 queries.

> Yes we probably should add a query that gets all the people that can see a
> merge proposal, although it is likely to be a chunky query.

I don't think it's a real priority unless the script starts running slowly.

Cheers,
mwh

« Back to merge proposal