[Bug 43826] avoid downloading the roster at every connection

[bugzilla-daemon at freedesktop.org]

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

--- Comment #6 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2012-01-19 09:51:16 PST ---
(In reply to comment #5)
> <andrunko> this is a programming error and should break code relying on it

The g_critical() will do that, if made fatal (we often do that).

> <andrunko> albanc, it actually does not matter, you could return whatever you
> want, the code is already buggy, but I think you should still return FALSE
> <andrunko> in every place a IS_SOMETHING_EXPECTED fails

I think you should return whichever value won't cause callers to do something
dangerous in response. In the case of "will it download the roster or not?",
either way is equally harmless, so my vote is "leave it as-is and get on with
your life".

> > > +typedef void (*TpBaseContactListFunc) (
> > > + TpBaseContactList *self,
> > > + GAsyncReadyCallback callback,
> > > + gpointer user_data);
> > > 
> > > I don't like this name? It seems way too vague.
> > 
> > I changed it to TpBaseContactListAsyncFunc. The other names (such as
> > TpBaseContactListActOnContactsFunc) are named after their signature only since
> > they are used for several purposes.

I started this convention with the initial implementation of TpBaseContactList,
and I'm inclined to agree with Alban:

> Rename it to TpBaseContactListDownloadFunc similar to
> TpBaseContactListRequestSubscriptionFunc?

Unlike the function for requesting subscriptions, nothing about that signature
is specific to downloading contact lists; a function of that signature could do
any action that doesn't have additional arguments.

What I was trying to avoid with ActOnContactsFunc is that we have one typedef
per virtual method, even though a lot of them are remarkably similar (like
ActOnContactsFunc, which does some verb (e.g. unblock) to a set of contacts).
It's the same reasoning as having GDestroyNotify instead of
GHashTableValueFreeFunc, GPtrArrayItemFreeFunc and GAsyncResultUserDataFreeFunc
:-)

(In reply to comment #2)
> - was it the correct place to add the changes in the Mutable interface?

I don't really see why this is in the Mutable interface: whether we can delay
downloading the contact list has nothing to do with whether we can modify it.
Mutable is about whether the contact list can be modified from the Telepathy
side - for instance, in principle it should be missing from Facebook contact
lists, because Facebook's XMPP server doesn't allow any edits.

Delayed-download works equally well on an immutable contact list, so I think
these new things should replace an appropriate amount of ABI padding in
TpBaseContactList itself.

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