Bug 46435 - Call1 log support
Summary: Call1 log support
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: logger (show other bugs)
Version: unspecified
Hardware: Other All
: medium enhancement
Assignee: Telepathy bugs list
QA Contact: Telepathy bugs list
URL: http://cgit.collabora.com/git/user/ni...
Whiteboard:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2012-02-22 01:55 UTC by Guillaume Desmottes
Modified: 2012-03-28 18:21 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments

Description Guillaume Desmottes 2012-02-22 01:55:03 UTC
For GNOME 3.4 we'd need a release of logger supporting Call1. We should also mark this feature as stable as it used be experimental and optionally enabled.
Comment 1 Nicolas Dufresne 2012-02-22 12:09:08 UTC
Branch is nearly ready. The only issue I'm facing is that an enum has been extended changing significantly the resulting error codes.

As Call was unstable, I'm temped to simply set the error as they are in the new Spec, and leave it to the UI to support the new enum values. Does that make sense ?
Comment 2 Guillaume Desmottes 2012-02-23 01:48:52 UTC
Sounds good to me. Calls logging was experimental and had to be manually enabled so I don't really care if we break old logged calls.
Comment 3 Nicolas Dufresne 2012-03-28 15:47:50 UTC
Branch ready for review.
Comment 4 Jonny Lamb 2012-03-28 16:38:44 UTC
The diff's kind of hard to read so I only half looked at it for call-channel.c and just looked at the new file as is.

>  param_spec = g_param_spec_int ("end-reason",
>      "End Reason",
>      "Reason for wich this call was ended",
> -    0, G_MAXINT, TPL_CALL_END_REASON_UNKNOWN,
> +    0, G_MAXINT, TP_CALL_STATE_CHANGE_REASON_UNKNOWN,
>      G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY | G_PARAM_STATIC_STRINGS);

You could use TP_NUM_CALL_STATE_CHANGE_REASONS instead of G_MAXINT here if you wanted?

> - reasons[priv->end_reason],
> + _tpl_call_event_end_reason_to_str(priv->end_reason),

Missing space before the bracket; that happens a couple of times I think.

>  if (contact == NULL)
>      contact = tp_connection_get_self_contact (con);

Should be two spaces instead of one.

Do the calls to dbus_g_object_register_marshaller need to stay?

Other than those minor points it looks basically fine. I don't know much about the logger but I'm sure it'll be fine.
Comment 5 Nicolas Dufresne 2012-03-28 18:21:03 UTC
Fixed and merged.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.