launchpad-reviewers team mailing list archive

[Данило Шеган <danilo@xxxxxxxxxxxxx>]

The proposal to merge lp:~danilo/launchpad/translatedlanguage into lp:launchpad/devel has been updated.

Description changed to:

= ITranslatedLanguage =

This provides a generic ITranslatedLanguage interface for objects which are a translation of something (i.e. a productseries, distroseries, sourcepackage, template) into a single language, along with a mixin that implements this interface in a generic way.

Mixin is to replace most of the model code on DistroSeriesLanguage and ProductSeriesLanguage, and to be the basis of cleaning up SourcePackageTranslations.  It relies on the "parent" object implementing IHasTranslationTemplates with its getCurrentTemplatesCollection() method.

The next steps would be to switch ProductSeriesLanguage, DistroSeriesLanguage and SourcePackageTranslations to make the most of the mixin, but that would result in a huge branch (as if this one isn't big already).  So, we only migrate ProductSeriesLanguage in this one (other than a few display-orienteed attributes, the only bits that remain in it are IRosettaStats methods which we want to get rid of as well).

As a preparation for getting rid of IRosettaStats, I introduce a temporary statistics object implementation (a dict) which we want to switch everything to (a better one is in progress in one of Adi's branches).

The most interesting bit of the code is inside the mixin: POFilesByPOTemplates is an iterator-like object which allows slicing over a full set of POTemplates regardless of the presense of POFiles (when they are missing, we return DummyPOFile objects). This ensures we do a constant number of queries for every request.

Unfortunately, for listifying TranslatedLanguageMixin.pofiles when __len__ is defined on POFilesByPOTemplates (a requirement for BatchNavigator), it's always called even if using an iterator would be enough: this means 2 queries instead of 1.  However, slicing always does a single query, as confirmed in the test.

It is (somewhat) indirectly unit-tested inside the TranslationTemplateMixinTest, though that's simply because 'pofiles' attribute implementation in the mixin is basically a set-up of POFilesByPOTemplates.

Full test is otherwise written in a way to make it easy to extend for testing over different types of objects implementing ITranslatedLanguage, even though it only tests ProductSeriesLanguage now.

= Tests =

 bin/test -cvvt test_translatedlanguage -t serieslanguage

= Demo & QA =

A few examples:

 https://translations.launchpad.dev/evolution/trunk/+lang/es
 https://translations.launchpad.dev/evolution/trunk/+lang/es?batch=1
 https://translations.launchpad.dev/evolution/trunk/+lang/sr (no PO files)
 https://translations.launchpad.dev/evolution/trunk/+lang/sr?batch=1

And to confirm we haven't broken DistroSeriesLanguage pages:

 https://translations.launchpad.dev/ubuntu/hoary/+lang/es
 https://translations.launchpad.dev/ubuntu/hoary/+lang/es

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/registry/model/productseries.py
  lib/lp/testing/factory.py
  lib/lp/translations/browser/configure.zcml
  lib/lp/translations/browser/serieslanguage.py
  lib/lp/translations/configure.zcml
  lib/lp/translations/interfaces/potemplate.py
  lib/lp/translations/interfaces/productserieslanguage.py
  lib/lp/translations/interfaces/translatedlanguage.py
  lib/lp/translations/model/potemplate.py
  lib/lp/translations/model/productserieslanguage.py
  lib/lp/translations/model/translatedlanguage.py
  lib/lp/translations/tests/test_productserieslanguage.py
  lib/lp/translations/tests/test_translatedlanguage.py
  lib/lp/translations/tests/test_translationtemplatescollection.py

./lib/lp/translations/interfaces/potemplate.py
     736: E301 expected 1 blank line, found 2
     750: E301 expected 1 blank line, found 2
     784: E302 expected 2 blank lines, found 1
    1312: E202 whitespace before ']'
    1400: E202 whitespace before ']'
    1407: E202 whitespace before ']'
    1510: E202 whitespace before ']'

(E301 happens due to comments in interface definition, E202 because of multi-line list definitions; I am not changing these for now, though I did fix a bunch of lint issues; also, these are across two different files: interfaces/potemplate.py and model/potemplate.py, but linter is very buggy)


-- 
https://code.launchpad.net/~danilo/launchpad/translatedlanguage/+merge/30758
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~danilo/launchpad/translatedlanguage into lp:launchpad/devel.