Code review comment for lp:~mandel/ubuntu-download-manager/system-bus

Revision history for this message
Mike McCracken (mikemc) wrote : with system bus seems to work OK for me.
I was a little thrown by the parameters in progress_callback() being
backwards, though :)

really minor note - looks like the added calls to lastError() in
download_daemon.cpp don't add a space before they dump
"QDBUSError(whatever)", so maybe it'd be a good idea to pad the string
literal before them.

super nitpicky naming suggestion - initSessionBus() and
initSystemBus() sound like they are initializing the busses. Why not
name them after what they'll do, like openLogFile() and
openSyslogConnection() or something?

also, a note for the future branch where you add syslog - why not
separate generating the message (which looks the same for both cases)
from sending it to the logfile/syslog? logSystemMessage() and
logSessionMessage share a lot of code.

review: Approve

« Back to merge proposal