[Bug 20035] Avatar cache reference implementation

[bugzilla-daemon at freedesktop.org]

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

--- Comment #16 from Xavier Claessens <xclaesse at gmail.com> 2010-05-07 07:05:24 PDT ---
(In reply to comment #15)
> Does your branch pass "make check" with --enable-gtk-doc ?

Yes.

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

Fixed.

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

Fixed.

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

TpContact doesn't know about account, it only has a TpConnection.

> Log an error if mkdir failed?

Fixed

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

Fixed

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

Fixed

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

Fixed

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

For the avatar token, NULL means the CM doesn't know the token yet, waiting for
the server. "" means the CM knows that there is no avatar.

> +          g_warning ("Unknown feature %d", features[i]);
> Use WARNING

Fixed.

> Also, you changed the semantic as those function doesn't return any more in
> such case.

I don't think it's bad to continue with the valid features requested, should be
fine.

> I think it would be better to add a function after the old code adding more
> features if needed.

What do you mean?

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

I'll see if it's easy to write some.

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