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. _______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : [email protected] Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
