launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

Review: Approve code

Unless I'm forgetting something, this eliminates the penultimate delayed copy callsite, the last being the API's Archive.syncSource(s). Do you have a strategy for the final excision of that callsite and the significant volume of called code?


240	+ if dest_url is None:
241	+ dest_url = escape(
242	+ canonical_url(dest_archive) + '/+packages', quote=True)
243	+
244	+ if dest_display_name is None:
245	+ dest_display_name = escape(dest_archive.displayname)

This will probably double escape them; structured() escapes parameters itself. I'd also avoid talking about the escaping in the docstring, as one wouldn't normally expect a string to be passed through unescaped unless the docstring explicitly asked for HTML.

708	+ for copy in copied_publications:
709	+ self.logger.debug(copy.displayname)

What does the logging around this look like? AFAICT the job doesn't log much else, so it may seem somewhat of a non sequitur in the output.
-- 
https://code.launchpad.net/~cjwatson/launchpad/always-copy-packages-asynchronously-2/+merge/131928
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.