[Bug 21957] Doesn't implement SimplePresence

[bugzilla-daemon at freedesktop.org]

http://bugs.freedesktop.org/show_bug.cgi?id=21957





--- Comment #3 from jonner <jonathon at quotidian.org>  2009-10-05 19:58:11 PST ---
(In reply to comment #1)
> +get_contact_statuses(GObject *obj, const GArray *contacts, GError **error)
> ...
> +       if (!tp_handles_are_valid (handle_repo, contacts, FALSE, error))
> +       {
> +               return NULL;
> +       }
> 
> get_contact_statuses doesn't strictly need to check that, tp-glib should do so
> for us.

pushed a commit which removes this superfluous check

> +       /* FIXME: this should really be sent with lowest priority so it doesn't
> +        * block other messages */
> 
> Yes it probably should.

done

> +               if (status->optional_arguments) {
> +                       g_assert(status->index == IDLE_PRESENCE_AWAY);  /*
> optional args only for AWAY */
> 
> tp-glib does currently pass a NULL HT if there are no optional arguments in the
> status's spec, but that's a bit surprising. I don't think this assertion should
> be here; the index check can go into the if's condition.

ok, done.

> +               } else if (presence == IDLE_PRESENCE_OFFLINE) {
> +                       /* FIXME: send quit command? */
> +               }
> 
> We said that offline isn't user-settable, so this should not happen.

OK, i just made this a g_warn_if_reached()

> +       /* FIXME: do we really want to return TRUE here?  ideally, we'd check
> to see
> +        * if sending the AWAY command was successful... for now just pretend
> +        * everything succeeded */
> +       return TRUE;
> 
> In an ideal world we'd quietly retry if AWAY failed, and after a few attempts
> give up and set our locally-cached status back to what it was before.

I didn't address this at all yet.

> 
> +       if (presence != priv->presence ||
> +           away_msg != priv->status_msg) {
> 
> This is always true, because strings in C. :)

oh dear.  that was stupid.  fixed.

I pushed a new set of commits to my branch which address the comments above.


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