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

Revision history for this message
Manuel de la Peña (mandel) wrote :

> irl_tests.py 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?

Sure, lets do that is a goof idea and better now than never.

>
> 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.

Lets do it know, same reason as above.

« Back to merge proposal