launchpad-reviewers team mailing list archive

[Brad Crittenden <bac@xxxxxxxxxxxxx>]

Review: Approve code

Thanks for doing this John.

I understand your hesitance to not grow command line options at this time.  It does seem to be a bit crazy to have the current series repeated so often, even in the script class name and the name of the select variable.

I'd recommend

s/select_quantal/select_series/
s/WipeQuantalTranslationsScript/WipeNewSeriesTranslationsScript

and define

NEW_SERIES_NAME = 'quantal'

and use it in defining the select_series statement.

Otherwise this looks good.  If this is going to be an anointed script it might be good to add logic that it can only be run against the latest series.  What would happen, for instance, if it got run against Q after Q+1 got opened? Is this script non-destructive and idempotent?
-- 
https://code.launchpad.net/~jameinel/launchpad/fix-translations/+merge/110502
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.