launchpad-reviewers team mailing list archive

[Celso Providelo <celso.providelo@xxxxxxxxxxxxx>]

Review: Approve

Minor question inline about the new interface attribute declaration, but other than that it looks good for landing.

Thanks for sorting this.

Diff comments:

> === modified file 'lib/lp/soyuz/browser/configure.zcml'
> --- lib/lp/soyuz/browser/configure.zcml	2014-11-17 00:12:42 +0000
> +++ lib/lp/soyuz/browser/configure.zcml	2015-01-20 15:33:33 +0000
> @@ -678,7 +678,7 @@
>          />
>      <browser:url
>          for="lp.soyuz.interfaces.binarypackagerelease.IBinaryPackageReleaseDownloadCount"
> -        path_expression="string:+binaryhits/${binary_package_release/name}/${binary_package_release/version}/${binary_package_release/build/distro_arch_series/architecturetag}/${day}/${country/iso3166code2|string:unknown}"
> +        path_expression="string:+binaryhits/${binary_package_release/name}/${binary_package_release/version}/${binary_package_release/build/distro_arch_series/architecturetag}/${day}/${country_code}"
>          attribute_to_parent="archive"
>          />
>  
> 
> === modified file 'lib/lp/soyuz/interfaces/binarypackagerelease.py'
> --- lib/lp/soyuz/interfaces/binarypackagerelease.py	2014-04-24 06:36:22 +0000
> +++ lib/lp/soyuz/interfaces/binarypackagerelease.py	2015-01-20 15:33:33 +0000
> @@ -126,3 +126,8 @@
>          ReferenceChoice(
>              title=_('Country'), required=False, readonly=True,
>              vocabulary='CountryName', schema=ICountry))
> +
> +    country_code = TextLine(
> +        title=_("Country code"), required=True, readonly=True,

Shouldn't a readonly property be required=False ?

> +        description=_(
> +            'The ISO 3166-2 country code for this count, or "unknown".'))
> 
> === modified file 'lib/lp/soyuz/model/binarypackagerelease.py'
> --- lib/lp/soyuz/model/binarypackagerelease.py	2014-04-24 06:36:22 +0000
> +++ lib/lp/soyuz/model/binarypackagerelease.py	2015-01-20 15:33:33 +0000
> @@ -181,3 +181,11 @@
>      def binary_package_version(self):
>          """See `IBinaryPackageReleaseDownloadCount`."""
>          return self.binary_package_release.version
> +
> +    @property
> +    def country_code(self):
> +        """See `IBinaryPackageReleaseDownloadCount`."""
> +        if self.country is not None:
> +            return self.country.iso3166code2
> +        else:
> +            return "unknown"
> 
> === modified file 'versions.cfg'
> --- versions.cfg	2015-01-06 12:47:59 +0000
> +++ versions.cfg	2015-01-20 15:33:33 +0000
> @@ -151,9 +151,6 @@
>  zope.pagetemplate = 3.5.0-p1
>  # XXX: downgraded to avoid 3.9.2 cookie calculation changes
>  zope.session = 3.9.1
> -# XXX: downgraded to fix start failure due to BPRDC URL in 3.5.2
> -# (the fix for bug #1002242 is the problem)
> -zope.tales = 3.5.1
>  # p1 Build of lp:~mars/zope.testing/3.9.4-p1.  Fixes bugs 570380 and 587886.
>  # p2 With patch for thread leaks to make them skips, fixes windmill errors
>  #    with 'new threads' in hudson/ec2 builds.
> 


-- 
https://code.launchpad.net/~cjwatson/launchpad/zope.tales-upgrade/+merge/247030
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.