Code review comment for lp:~michael.nelson/launchpad/510331-syncsources-latest-pub

Revision history for this message
Graham Binns (gmb) wrote :

Hi Michael,

This looks good to me. One minor nitpick, but otherwise it's fine to
land as-is.

> === modified file 'lib/lp/soyuz/model/archive.py'
> --- lib/lp/soyuz/model/archive.py 2010-01-12 08:31:42 +0000
> +++ lib/lp/soyuz/model/archive.py 2010-01-26 11:54:18 +0000
> @@ -1121,6 +1112,23 @@
>
> self._copySources([source], to_pocket, to_series, include_binaries)
>
> + def _collectLatestPublishedSources(self, from_archive, source_names):
> + """Private helper to collect the latest published sources for an
> + archive.
> + """
> + sources = []
> + for name in source_names:
> + # Check to see if the source package exists, and raise a useful
> + # error if it doesn't.

What qualifies as "A useful error" here? It'd be nice to say in the
comment - or better still in the docstring - "will raise a FooBarError
if the source package doesn't exist."

> + getUtility(ISourcePackageNameSet)[name]
> + # Grabbing the item at index 0 ensures it's the most recent
> + # publication.
> + sources.append(
> + from_archive.getPublishedSources(
> + name=name, exact_match=True,
> + status=PackagePublishingStatus.PUBLISHED)[0])
> + return sources
> +
> def _copySources(self, sources, to_pocket, to_series=None,
> include_binaries=False):
> """Private helper function to copy sources to this archive.
>

review: Approve (code)

« Back to merge proposal