launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

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.