Code review comment for lp:~jtv/launchpad/bug-436426-2

Thanks for the review. Your remarks were:

> >=== modified file 'lib/lp/translations/doc/translationimportqueue.txt'
> >--- lib/lp/translations/doc/translationimportqueue.txt 2009-09-25 15:40:58
> +0000
> >+++ lib/lp/translations/doc/translationimportqueue.txt 2009-09-30 19:25:02
> +0000

> >@@ -1296,6 +1304,15 @@
> > If such bad requests do end up on the import queue, the import queue code
> will
> > raise errors about them.
> >
> >+ >>> def print_import_failures(import_script):
> >+ ... """List failures recorded in import script instance."""
>
> Should that be singular with an article:
> "in an/the import script instance"
> or plural without an article:
> "in import script instances"

"A." Added.

> >=== modified file 'lib/lp/translations/scripts/po_import.py'
> >--- lib/lp/translations/scripts/po_import.py 2009-09-18 07:39:51 +0000
> >+++ lib/lp/translations/scripts/po_import.py 2009-09-30 19:25:02 +0000

> >- def run(self):
> >- """Execute the import of entries from the queue."""
> >- # Get the queue.
> >+ # Time goal for one run. It is not exact. The script can run for
> >+ # longer than this, but will know to stop taking on new work.
> >+ # Since the script is run every 9 or 10 minutes, we set the "alarm"
>
>
> I would avoid calling this an alarm, since it makes me wonder if
> you are using signal.alarm(), even though you explain how it really
> works in the preceding sentence.

Okay, I changed it to say goal instead.

> >+ # at 8 minutes. That leaves a bit of time to complete the last
> >+ # ongoing batch of imports.
> >+ time_to_run = timedelta(minutes=8)
> >+
> >+ # Failures to be reported in bulk at closing, so we don't accumulate
> >+ # thousands of oopses for repetitive errors.
> >+ failures = None
>
>
> What is the purpose of initializing it here in addition to in the
> constructor? Is it used to create documentation that knows about
> instance variables?

Yes, just a place to comment it in really. Blame my background in static
languages.

> >+ self._reportFailures()
> >+
>
>
> Trailing white space.

Fixed. Thanks for spotting it.

Jeroen