Code review comment for lp:~mandel/ubuntu-download-manager/correct-logging

Revision history for this message
Loïc Minier (lool) wrote :

So this is already in, but I think it's unfortunate that some changes are hidden deep there both in terms of the way commits are split and the merge commit message; I've noticed that r153:
* changes level of a bunch of messages
* changes logging macros
* drops a bunch of messages

but also:
* switches to latin1 (this was reverted in r154)
* fixes support for warning level in messages sent to syslog()
* fixes path of log file
* renames onlineStateChange to onlineStateChanged

Which is not great when trying to review a larger diff like this one.

Generally ok with the changes, except I dont see how LP #1241005 is fixed since we flipped back and forth and we're back to the original approach?

I also wonder whether download-manager ought to escape things sent to syslog; what happens with e.g. logs with newlines?

« Back to merge proposal