[Bug 27271] API review: TplLogEntry, TplLogEntryText

[bugzilla-daemon at freedesktop.org]

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

Simon McVittie <simon.mcvittie at collabora.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                URL|http://git.collabora.co.uk/ |
                   |?p=user/kalfa/telepathy-log |
                   |ger.git;a=shortlog;h=refs/h |
                   |eads/log-entry-api-review   |

--- Comment #12 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2010-05-28 06:59:05 PDT ---
There might be implementations of some of these in:

http://git.collabora.co.uk/?p=user/kalfa/telepathy-logger.git;a=shortlog;h=refs/heads/log-entry-api-review

but it'll probably need significant merging.

> TplContact * tpl_log_entry_get_sender (TplLogEntry *self);
> TplContact * tpl_log_entry_get_receiver (TplLogEntry *self);

These are Bug #27269, and are out of scope here.

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

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

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.

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