launchpad-reviewers team mailing list archive

[John A Meinel <john@xxxxxxxxxxxxxxxxx>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 6/15/2012 10:19 PM, Brad Crittenden wrote:
> 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.

I went with 'series_name' since all the other variables were lowercase.

> 
> 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?
> 

Honestly, I'm not really sure how to detect what is the latest series.
I'm guessing there is a table somewhere that defines the known series,
along with some sort of ordering (or we can just sort() them and
assume alphabetical for now.)

Thanks for the review.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk/fCTEACgkQJdeBCYSNAANxWwCfaOHbS9RQDuNZTy6D6xaoDMNr
V5IAoKysESxlOSlni7h9ARfSP6zH/S0X
=nODJ
-----END PGP SIGNATURE-----

-- 
https://code.launchpad.net/~jameinel/launchpad/fix-translations/+merge/110502
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.