Code review comment for lp:~cjwatson/launchpad/always-copy-packages-asynchronously-2

Revision history for this message
William Grant (wgrant) wrote :

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.

review: Approve (code)

« Back to merge proposal