[Bug 43826] avoid downloading the roster at every connection

[bugzilla-daemon at freedesktop.org]

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

--- Comment #7 from Andre Moreira Magalhaes <andrunko at gmail.com> 2012-01-19 10:03:34 PST ---
(In reply to comment #6)
> (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".
Agreed, just leave it as-is.

> > > > +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
> :-)
Makes sense, I have no preference here tbh, either way I am fine.

> (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.
Hmm now the "Mutable" name makes sense to me, how did I miss that? After
reading this rationale I have to agree that it should be 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.