[Bug 37358] TpChannel: high level API for SMS

[bugzilla-daemon at freedesktop.org]

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

--- Comment #6 from Guillaume Desmottes <guillaume.desmottes at collabora.co.uk> 2011-05-24 02:28:27 PDT ---
(In reply to comment #5)
> -      destroyable_iface_init)
> +      destroyable_iface_init);
> +    G_IMPLEMENT_INTERFACE (TP_TYPE_SVC_CHANNEL_INTERFACE_SMS, sms_iface_init);
> 
> These macros shouldn't end with a ; because it breaks some compilers (ask smcv
> for details).

Right, I already experienced that as well; removed.

> +set_property (GObject *object,
> +              guint property_id,
> +              const GValue *value,
> +              GParamSpec *pspec)
> 
> Not in Telepathy style.

fixed.

> +      case PROP_SMS:
> +        self->priv->sms = g_value_get_boolean (value);
> +        break;
> +    default:
> +      G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec);
> +      break;
> 
> Weird indenting.
> 
> Similarly both for get_property()

fixed.

> +  static TpDBusPropertiesMixinIfaceImpl prop_interfaces[] = {
> +      { TP_IFACE_CHANNEL_INTERFACE_SMS,
> +        tp_dbus_properties_mixin_getter_gobject_properties,
> +        NULL,
> +        sms_props,
> +      },
> +      { NULL }
> +  };
> 
> +  klass->dbus_properties_class.interfaces = prop_interfaces;
> +  tp_dbus_properties_mixin_class_init (object_class,
> +      G_STRUCT_OFFSET (ExampleEcho2ChannelClass, dbus_properties_class));
> 
> TpBaseChannel already implements TpSvcDBusProperties, you can use
> tp_dbus_properties_mixin_implement_interface()

Oh good point; done.

> Personally, I don't think SMS needs to be a separate feature, that it's worth
> just having in CORE.

The general philosophy is to restrict CORE as much as possible. Users won't
have to care about the SMS feature as most of them will use the default
factory preparing this iface (actually I forgot to add it but I did now. :)

> +  param_spec = g_param_spec_boolean ("is-sms",
> 
> is-sms-channel ?

renamed.

> +  if (error != NULL)
> +    {
> +      DEBUG ("Failed to get SMS length: %s", error->message);
> +
> +      g_simple_async_result_set_from_error (result, error);
> +    }
> 
> missing goto finally; ?

Ooops; fixed.

http://cgit.collabora.com/git/user/cassidy/telepathy-glib/log/?h=sms-37358
updated

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