On Wed, Dec 07, 2011 at 07:20:29AM -0000, Jeroen T. Vermeulen wrote: > The part from “series = None” onward seems to be an isolated unit of > work. I think it looks for the first distroseries in development > state, or failing that, the first in frozen state. But the structure > of the code makes that hard to figure out, and then only afterwards I > can start wondering why you do it exactly this way. > > I fervently recommend extracting that code into a sensibly-named > function. (It doesn't need to be a method: AFAICS a distribution goes > in, a series comes out, and that is the full extent of its interaction > with the rest of the world). Come to think of it, might there already > be a suitable method in Distribution somewhere?
Fair comment. Distribution.currentseries is nearly there: it just sometimes returns series in statuses we don't want here; but if there's one we can use then it will always return it, so we can just call currentseries and then check the result. > * For Python strings I find consistent double quotes a good habit, at > least for free-form texts that may just as well contain an > apostrophe. This is an out-of-context habit of mine from Perl and shell programming: since single and double quotes have different interpolation characteristics there, I've trained myself to use single quotes unless either I have an explicit reason to want interpolation or the string contains an apostrophe and no double quotes. I realise this doesn't make as much sense for Python, so I'll go through and amend this. -- https://code.launchpad.net/~cjwatson/launchpad/refactor-cron-germinate/+merge/84624 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/refactor-cron-germinate into 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
