[Bug 29981] Support power saving interface

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=29981

Eitan Isaacson <eitan.isaacson at collabora.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Status Whiteboard|review-, minor changes      |

--- Comment #9 from Eitan Isaacson <eitan.isaacson at collabora.co.uk> 2010-09-09 12:26:57 PDT ---
(In reply to comment #6)
> Aside from my/Simon's comments above, just trivia about the updated code.
> 
> In toggle_queueing_cb():
> 
> +  DEBUG (" ");
> +
> 
> Surely better just to log something in both branches of:
> 
> +  if (error)
> 
> (which should be "if (error != NULL)", or actually "if (reply == NULL)". Ditto
> later in the file: if you check the reply's NULL-ness, you don't need the extra
> conditional as to whether to unref it).
> 

cc39e14

> Hmm also, if we've got a stanza in flight to enable queueing, we probably don't
> want to send a second one redundantly... but then this gets complicated. What
> happens if someone calls SetPowerSaving(True), SPS(False), SPS(True), all
> before the server responds to the first one. Probably not worth it.

Yeah, although no real harm would come. It would just be stupid.

> 
> +    gboolean in_Active,
> 
> This is a misleading name for the argument. The code-genned examples shouldn't
> include the in_, really. But in this case it not only looks ugly, but means the
> opposite of what was intended!

f18e130

> 
> I think the GabbleConnection should have a "power-saving" property... and then
> we could implement the D-Bus property using
> tp_dbus_properties_mixin_getter_gobject_properties(). Much less faffing around
> with quarks.

3a863ce

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.