[Bug 20035] Avatar cache reference implementation

[bugzilla-daemon at freedesktop.org]

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

--- Comment #15 from Guillaume Desmottes <guillaume.desmottes at collabora.co.uk> 2010-05-07 06:13:01 PDT ---
Does your branch pass "make check" with --enable-gtk-doc ?

 * Returns: the same file as the #TpContact:avatar-file property
"the same #GFile"

You should use the same phrasing when documenting the lifetime of the returned
file ("This remains valid until the main loop...").

  dir = g_build_filename (g_get_user_cache_dir (),
      "telepathy", "avatars", cm_name, protocol, NULL);
Wouldn't it be better to use the account rather than the connection to build
this directory?

Log an error if mkdir failed?

_build_filename would be a better name IMHO as it's doesn't get an existing
filename.

  DEBUG ("Contact#%u avatar stored in cache: %s, %s", handle, filename,
mime_type);
This line is too long.

You should check the return value of g_file_set_contents and log an error if it
fails.

contact_maybe_request_avatar_data
this check looks weird: self->priv->avatar_token[0] == '\0'
Why isn't NULL in that case?

+          g_warning ("Unknown feature %d", features[i]);
Use WARNING
Also, you changed the semantic as those function doesn't return any more in
such case.
I think it would be better to add a function after the old code adding more
features if needed.


No having tests makes me cry, especially as TpContact is pretty well tested so
far. I let smcv decide if that's a merge blocker or not.

-- 
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.