[Bug 33410] Do on-disk avatar cache in CM
bugzilla-daemon at freedesktop.org
bugzilla-daemon at freedesktop.org
Thu Jun 7 17:42:43 CEST 2012
https://bugs.freedesktop.org/show_bug.cgi?id=33410
--- Comment #14 from Jonny Lamb <jonny.lamb at collabora.co.uk> 2012-06-07 08:42:43 PDT ---
Here are some comments about your tp-glib branch:
* the docs at the top of TpAvatarMixin are confusing. You start off with
"To use the avatars mixin" then a section "Implementing Avatars". Try and
consolidate the two.
+ /* TpHandle -> AvatarData
Owned AvatarData? Please make it explicit.
+ * <informalexample><programlisting>
+ * tp_avatars_mixin_init ((GObject *) self,
+ * G_STRUCT_OFFSET (SomeObject, avatars_mixin));
+ * </programlisting></informalexample>
tp_avatars_mixin_init has loads more parameters than that.
+ static GQuark q[NUM_MIXIN_DBUS_PROPERTIES] = { 0, };
+
+ if (G_UNLIKELY (q[0] == 0))
+ {
I don't like this. Why don't you just use the getter data in
TpDBusPropertiesMixinPropImpl? So something like this:
static TpDBusPropertiesMixinPropImpl known_avatars_props[] = {
{ "AvatarPersists", GUINT_TO_POINTER (MIXIN_DP_AVATAR_PERSISTS), NULL },
...
then you can just check the GPOINTER_TO_UINT-ed value in the getter function?
Include the standard TP_BASE_CONNECTION_ERROR_IF_NOT_CONNECTED in the
RequestAvatars implementation.
g_file_get_contents is always bad. avatar_cache_lookup is only called by
tp_avatars_mixin_avatar_changed so we could make it use the async version, no?
Same for _set_contents?
(I didn't look at your other patches, updating TpContact and the unit tests.)
(In reply to comment #13)
> * why is there a AvatarsNeedRequest signal? (that name sounds bad too, btw)
> Why doesn't the CM just request the avatar and save it in the cache
> immediately?
Actually I can kind of see how if a client doesn't care about avatars it won't
want this unnecessary network traffic. I propose a rw boolean property instead
(which is also a D-Bus connection parameter) which determines whether avatars
will be automatically requested or not. Y/N
--
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.
More information about the telepathy-bugs
mailing list