[Bug 32477] Log Call events

[bugzilla-daemon at freedesktop.org]

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

--- Comment #15 from Nicolas Dufresne <nicolas.dufresne at collabora.co.uk> 2011-03-24 14:27:05 PDT ---
(In reply to comment #13)
> (In reply to comment #12)
> > (In reply to comment #11)
> > > +/* -*- Mode: C; tab-width: 8; indent-tabs-mode: t; c-basic-offset: 8 -*- */
> > > 
> > > Not really (in lots of files).
> > 
> > Right, it seems wrong, I don't really know which editor uses that, this has
> > been copy-pasted over-and-over. Shall we simply remove them all ?
> 
> I think vi (or Emacs), but the problem is that it's wrong, that says the coding
> style uses tabs, but we use two spaces... so just get rid of them everywhere,
> if they are like that.

Ok, removed.

> 
> > > +static void
> > > +tpl_call_event_finalize (GObject * obj)
> > > +{
> > > +  G_OBJECT_CLASS (tpl_call_event_parent_class)->finalize (obj);
> > > +}
> > > 
> > > Don't override finalize for nothing.
> > 
> > Hmm, made me notice I don't free anything. Replaced with _dispose and
> > now tp_clear_object() and tp_clear_pointer() the end_actor and
> > detailed_end_reason. Will run this under valgrind and fix remaining if they
> > exists.
> 
> make check-valgrind could be useful here.

I've run it, this was the only leak the unit tests covers.


> > > +        tpl_entity_get_avatar_token (actor) == NULL ?
> > > +           "" : tpl_entity_get_avatar_token (actor),
> > > 
> > > I guess you should escape that too.
> > 
> > escape ? that too ? Maybe you mean it's not required since I now create entity
> > with "" as default avatar_token ? I agree, but it's also an extra safety in the
> > case somebody break it in the future.
> 
> I said "too" because a few lines above you escape another avatar-token. So
> either escape both or don't escape any, I'd say.

Ah escaping in order to put it in the XML file, make sense. I also removed the
if NULL for avatar token and alias has this is no longer possible.

> 
> > > +++ b/telepathy-logger/call-channel.c
> > > +G_DEFINE_TYPE_WITH_CODE (TplCallChannel, _tpl_call_channel,
> > > +    TP_TYPE_CHANNEL,
> > > +    G_IMPLEMENT_INTERFACE (TPL_TYPE_CHANNEL, tpl_call_channel_iface_init))
> > > 
> > > Have you thought about inheriting from TpyCallChannel instead as you did with
> > > TpTextChannel? That could ease our job.
> > 
> > Yeah, as I already explained, I pretty much dislike the submodules and having
> > unstable static libraries being put in more then 1 project. I'll port it, but
> > unstable library like Yell are not my thing.
> 
> Yes, ideally it'd be in tp-glib (which will be when it becomes stable). The
> stability point shouldn't be too important as we're not breaking it unless we
> bump the draft, in which case it's a different channel and you're not logging
> them anymore.

I tried, but it didn't work, I've stored the work in a branch and I'll come
back later to see what's wrong with TpyCallChannel implementation.

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