[Bug 27271] clean up TplLogEntry, TplLogEntryText API

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Mon Jun 7 16:43:17 CEST 2010


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

--- Comment #14 from Guillaume Desmottes <guillaume.desmottes at collabora.co.uk> 2010-06-07 07:43:16 PDT ---
http://git.collabora.co.uk/?p=user/cassidy/telepathy-logger;a=shortlog;h=refs/heads/entry-27271

(In reply to comment #13)
> Another trivial thing: I wonder if this class should be called TplEntry, since
> the entire library is about logs :-)

renamed.

> >   param_spec = g_param_spec_uint ("signal-type",
> >       "SignalType",
> >       "The signal type which caused the log entry",
> 
> This is public API despite not being in the header, because it's a property.
> The enum that lets you interpret it is not public API. Am I right in thinking
> that Empathy silently ignores every TplLogEntry that isn't a TplLogEntryText?
> That seems less than ideal, to say the least!

I removed this property as it's only used by the logger and not Empathy.

> > gint tpl_log_entry_get_pending_msg_id (TplLogEntry *self);
> 
> This is specific to Text and should be in the subclass; KA's branch made this
> change.
> 
> See Bug #26838 for some thoughts on safety (or lack of) of using pending
> message IDs to try to construct unique identifiers for messages.
> 
> What does Empathy use this for?

Empathy creates an EmpathyMessage from the log entry and uses this as message
id:

    empathy_message_set_id (retval,
            tpl_log_entry_get_pending_msg_id (logentry));


> This is a writeable property, which means library users can change it
> arbitrarily - that seems dangerous. Make it read-only, with a library-internal
> setter function, instead. timestamp, signal-type, chat-id, channel-path, sender
> and receiver have the same bug, with the same solution.
> 
> >   /**
> >    * TplLogEntry::log-id:
> >    *
> >    * A token which can be trusted as unique over time within TPL.
> 
> Not really :-P See Bug #26838.
> 
> >   param_spec = g_param_spec_uint ("direction",
> >       "Direction",
> >       "The direction of the log entry (in/out)",
> 
> Derivable from whether the sender is self. This is public API because it's a
> property, but it's not useful because the enum isn't public API.

Moved TplEntryDirection to the API.

> > const gchar *tpl_log_entry_text_get_message (TplLogEntryText *self);
> 
> This isn't even sufficient for the old Text interface, which also carried a
> TpChannelTextMessageType and TpChannelTextMessageFlags. At the very least,
> Empathy needs to be able to ignore "empty" messages with the Non_Text_Content
> flag.

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



More information about the telepathy-bugs mailing list