Code review comment for lp:~jcsackett/launchpad/convert-sql-627631

Hi JC,

I found a couple of things that should be changed in your incremental diff.

>=== modified file 'lib/lp/registry/model/projectgroup.py'
>--- lib/lp/registry/model/projectgroup.py 2010-11-02 20:10:56 +0000
>+++ lib/lp/registry/model/projectgroup.py 2010-11-04 12:54:30 +0000
>
> def has_translatable(self):
> """See `IProjectGroup`."""
>@@ -205,7 +201,7 @@
> # converted to use is_empty but the implementation in storm's
> # sqlobject wrapper is broken.
> # return not self.translatables().is_empty()

It looks like this comment can also be removed now.

>- return self.translatables().count() != 0
>+ return self.translatables().is_empty()
>
> def has_branches(self):
> """ See `IProjectGroup`."""
>
>=== modified file 'lib/lp/translations/scripts/translations_to_branch.py'
>--- lib/lp/translations/scripts/translations_to_branch.py 2010-10-02 11:41:43 +0000
>+++ lib/lp/translations/scripts/translations_to_branch.py 2010-11-04 01:48:10 +0000
>@@ -306,15 +306,13 @@
>
> self.store = getUtility(IStoreSelector).get(MAIN_STORE, SLAVE_FLAVOR)
>
>- # XXX j.c.sackett 2010-08-30 bug=627631 Once data migration has
>- # happened for the usage enums, this sql needs to be updated to
>- # check for the translations_usage, not official_rosetta.
> product_join = Join(
> ProductSeries, Product, ProductSeries.product == Product.id)
> productseries = self.store.using(product_join).find(
>- ProductSeries, SQL(
>- "official_rosetta AND translations_branch IS NOT NULL"))
>-
>+ ProductSeries,
>+ AND(
>+ Product.translations_usage == ServiceUsage.LAUNCHPAD,
>+ Product.translations_branch is not None))

You can't use "is not None" for a storm conditional, since python does
not allow that operator to be overridden. Instead use
                Product.translations_branch != None
and storm will be smart enough to change it to "IS NOT NULL".