Thanks for the review! I was wondering when the LoC thing would come up. :) In defense of Francis' note, not entirely without self-interest, I'm also obviating considerable command-lining and human-scripting (“disable X, click Y”) for every Ubuntu cycle.
> Looks good. One suggestion: > > Instead of > > 13 + if template_id not in cached_potmsgsets: > 14 + cached_potmsgsets[template_id] = get_potmsgset_ids(template_id) > 15 + potmsgset_ids = cached_potmsgsets[template_id] > > I think you can just do > > 13 + potmsgset_ids = cached_potmsgsets.setdefault(template_id, > get_potmsgset_ids(template_id)) That would evaluate get_potmsgset_ids(template_id) before the setdefault() call. There's not a very great distance between the call constructing a query that will be executed only when needed, and it also executing the query in order to return some other data structure. So it would make the caching a bit brittle! Does make me wonder if dict.setdefault shouldn't accept a callable plus arguments so that it can evaluate lazily. Jeroen -- https://code.launchpad.net/~jtv/launchpad/bug-994650-cache-potmsgsets/+merge/105193 Your team Launchpad code reviewers is subscribed to branch lp:launchpad. _______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : [email protected] Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
