Code review comment for lp:~adiroiban/launchpad/bug-146178

Hi Adi,

Thanks for submitting this change. The work looks really good.

Please fix the small items below and then I'll (try) to land it for you, if ec2 will let me. :(

> === modified file 'lib/lp/registry/interfaces/distroseries.py'
> --- lib/lp/registry/interfaces/distroseries.py 2010-04-08 18:40:18 +0000
+++ lib/lp/registry/interfaces/distroseries.py 2010-04-21 22:30:59 +0000
> @@ -250,33 +250,33 @@
> language_pack_base = Choice(
> title=_('Language pack base'), required=False,
> description=_('''
> - Language pack export with the export of all translations available
> - for this `IDistroSeries` when it was generated. Next delta exports
> - will be generated based on this one.
> + Language pack with the export of all translations
> + available for this distribution series when it was generated. The
> + subsequent update exports will be generated based on this one.
> '''), vocabulary='FilteredFullLanguagePack')
>
> language_pack_delta = Choice(
> - title=_('Language pack delta'), required=False,
> + title=_('Language pack update'), required=False,
> description=_('''
> - Language pack export with the export of all translation updates
> - available for this `IDistroSeries` since language_pack_base was
> - generated.
> + Language pack with the export of all translation updates
> + available for this distribution series since the language pack
> + base was generated.
> '''), vocabulary='FilteredDeltaLanguagePack')
>
> language_pack_proposed = Choice(
> title=_('Proposed language pack update'), required=False,
> description=_('''
> - Base or delta language pack export that is being tested and
> - proposed to be used as the new language_pack_base or
> - language_pack_delta for this `IDistroSeries`.
> + Base or update language pack export that is being tested and
> + proposed to be used as the new language pack base or
> + language pack update for this distribution series.
> '''), vocabulary='FilteredLanguagePack')
>
> language_pack_full_export_requested = Bool(
> title=_('Request a full language pack export'), required=True,
> description=_('''
> Whether next language pack generation will be a full export. This
> - is useful when delta packages are too big and want to merge all
> - those changes in the base package.
> + is useful when update packs are too big and want to merge all
> + those changes in the base pack.

s/This is/This information is/

> '''))
>
> last_full_language_pack_exported = Object(

> === modified file 'lib/lp/translations/browser/distroseries.py'
> --- lib/lp/translations/browser/distroseries.py 2010-02-24 18:33:18 +0000
> +++ lib/lp/translations/browser/distroseries.py 2010-04-21 22:30:59 +0000
> @@ -122,6 +122,28 @@
>
> return unused_language_packs
>
> + @property
> + def have_latest_full_pack(self):

Please add a docstring here.

> + current = self.context.language_pack_base
> + latest = self.context.last_full_language_pack_exported
> + if (current is None or
> + latest is None or
> + current.file.http_url == latest.file.http_url):
> + return False
> + else:
> + return True
> +
> + @property
> + def have_latest_delta_pack(self):

And here.

> + current = self.context.language_pack_delta
> + latest = self.context.last_delta_language_pack_exported
> + if (current is None or
> + latest is None or
> + current.file.http_url == latest.file.http_url):
> + return False
> + else:
> + return True
> +
> def _request_full_export(self):
> if (self.old_request_value !=
> self.context.language_pack_full_export_requested):

> === modified file 'lib/lp/translations/templates/distroseries-language-packs.pt'
> --- lib/lp/translations/templates/distroseries-language-packs.pt 2009-09-17 11:10:49 +0000
> +++ lib/lp/translations/templates/distroseries-language-packs.pt 2010-04-21 22:30:59 +0000