[Merge] lp:~tonyyarusso/update-notifier/bug-1167621 into lp:update-notifier

Martin Pitt martin.pitt at ubuntu.com
Tue May 21 17:27:29 UTC 2013


Review: Needs Fixing

8       -def write_package_names(outstream, cache, depcache):
9	+def write_package_names(cache, depcache, messages, standalone, delim="\n"):
10	" write out package names that change to outstream "

Can you please adjust the docstrings accordingly? Also, you should rename the methods, as they don't write anything any more. Perhaps get_package_names()? Also, it seems this function doesn't actually use the new "standalone" argument? 

For write_human_readable_summary, please also document the two new arguments in the docstring. I think it's an unusual style to pass in "messages" and have the function return that plus some extra text; instead, concatenate the result of the two functions in the caller.

64	+ "--delimeter",

It's "delimiter" (same typo again further down).

Thanks!
-- 
https://code.launchpad.net/~tonyyarusso/update-notifier/bug-1167621/+merge/158500
Your team Ubuntu Core Development Team is requested to review the proposed merge of lp:~tonyyarusso/update-notifier/bug-1167621 into lp:update-notifier.



More information about the Ubuntu-reviews mailing list