Code review comment for lp:~pitti/indicator-session/libupower-glib

Revision history for this message
Ted Gould (ted) wrote :

There are a few issues with the patch.

It uses the _sync calls, but we have a requirement to make all DBus
operations async in the various indicator code. I'm unclear whether
up_client_get_can_suspend is sync or async.

I generally prefer to check for all the libraries at once in
configure.ac. I find that this frustrates users less as they get all
the errors at once instead of getting one error, the next error, as they
go through the configure script.

It should really use g_strcmp0 instead of strcmp as it protects from
NULL strings which will otherwise be crashers.

It seems that the screensaver_unthrottle() was removed from after the
sleep is issued, and I don't see where it was re-added.

  review needs-info

review: Needs Information

« Back to merge proposal