launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

Review: Approve code

20	+ def _allIndexFiles(self, distroseries):
21	+ """Return all index files on disk for a distroseries."""

This method seems like it should mostly be elsewhere. It's not reasonably practical to factor out the commonalities somehow?

81	+ if pocket == PackagePublishingPocket.RELEASE:
82	+ alias_suite = alias
83	+ else:
84	+ alias_suite = "%s%s" % (alias, pocketsuffix[pocket])

This need not be special-cased; pocketsuffix[RELEASE] == "".

57	+ def createSeriesAliases(self):

This has the slightly weird behavior that a single missing current suite will cause just that alias to point to the old series, while the other suites point to the new one. This probably can't happen in production, because primary archive indices are always all generated for all suites on series creation, even if empty.

732	distribution = self.distribution
733	if to_series is not None:
734	result = getUtility(IDistroSeriesSet).queryByName(
735	- distribution, to_series)
736	+ distribution, to_series, follow_aliases=True)
737	if result is None:
738	raise NoSuchDistroSeries(to_series)
739	series = result

This is probably fairly confusing for PPAs. You can copy to devel and it'll actually be different from the current devel symlink in the PPA, with no way to tell beforehand. But we probably can't avoid it given the current fairly braindead method signature.
-- 
https://code.launchpad.net/~cjwatson/launchpad/series-alias/+merge/178103
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.