[Bug 33015] Port to TpTextChannel

[bugzilla-daemon at freedesktop.org]

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

--- Comment #3 from Nicolas Dufresne <nicolas.dufresne at collabora.co.uk> 2011-03-21 10:20:01 PDT ---
(In reply to comment #2)
> telepathy-logger/text-channel.c:
> 
> +  else if (!tp_proxy_has_interface_by_id (TP_PROXY (chan),
> +        TP_IFACE_QUARK_CHANNEL_INTERFACE_MESSAGES))
> +    {
> +      error = g_error_new (TPL_TEXT_CHANNEL_ERROR,
> +          TPL_TEXT_CHANNEL_ERROR_NEED_MESSAGE_INTERFACE,
> +          "The text channel does not implement Message interface.");
> +      _tpl_action_chain_terminate (ctx, error);
> +      g_error_free (error);
> +    }
> +
> +  _tpl_action_chain_continue (ctx);
> +}
> 
> You should return after g_error_free

Oops, will fix.

> 
> 
> I don't like
> http://git.collabora.co.uk/?p=user/nicolas/telepathy-logger.git;a=commitdiff;h=18be97017eb658b14c5069bb17c03145f8dc3eb4
> 
> You should still print that the event may not have been logged. To not crash,
> you can do loc_error ? loc_error->message : "something", or an if (loc_error)
> else, or whatever, but still do a CRITICAL.

My logic was the if you get that, it's because you have hit a
g_return_val_if_fail() which print stuff in the critical level anyway. I
personally find it annoying when too many errors get displayed for the same
thing. But I don't mind doing it this way, and clearly state that an event was
lost.

> 
> Building it:
> text-channel.c: In function ‘pendingproc_store_pending_messages’:
> text-channel.c:568:26: error: ‘cached’ may be used uninitialized in this
> function
> 
> The easy fix is to bring cached = cached_it->data; a bit up.

Oops! I've moved the initialization in between the two if. This is indeed a
really rare case, I'm trying to produce that case at the moment, but it's hard
to do manually. 

> 
> Most of my other comments are fixed in later commits. I'm afraid of the
> regressions this could bring, but the test suite passes, so if something
> breaks, we know the test suite is not as good as we would like it to be and
> need to improve it.

Well I can guaranty tests need's improvement, but I'm still removing 2K lines
of potentially untested code.

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