+GType
+salut_plugin_get_type (void)
+{
+  static GType type = 0;
+
+  if (type == 0) {
+    static const GTypeInfo info = {
+      sizeof (SalutPluginInterface),
+      NULL,   /* base_init */
+      NULL,   /* base_finalize */
+      NULL,   /* class_init */
+      NULL,   /* class_finalize */
+      NULL,   /* class_data */
+      0,
+      0,      /* n_preallocs */
+      NULL    /* instance_init */
+    };
+
+    type = g_type_register_static (G_TYPE_INTERFACE, "SalutPlugin", &info, 0);
+  }
+
+  return type;
+}

This can be G_DEFINE_INTERFACE() since 2.24 I think.

(Reviewed up to “plugin-loader: add plugin loader from gabble”. This code is all quite familiar.)

+  g_ptr_array_foreach (tmp, add_to_array, managers);
+  g_ptr_array_free (tmp, FALSE);

+      g_ptr_array_foreach (managers, copy_to_other_array, out);
+      g_ptr_array_free (managers, TRUE);

Same review comment as for the corresponding code in Gabble.


+/* The return type should be a new GPtrArray* which will be freed
+ * straight after this function is called, so the pointer array must
+ * not have a free function. */

Really, you should say that the caller takes ownership of the array and of the references to channel managers within it, and maybe refer to TpBaseConnectionCreateChannelManagersImpl which has the same semantics?

“Protocol: allow the name, icon name and English to be set easier”

Nice troll!


-  name = g_strdup_printf ("%s." SALUT_DNSSD_PRESENCE ".local", _self->name);
+  const gchar *dnssd_name;
+
+  dnssd_name = salut_avahi_discovery_client_get_dnssd_name (
+      priv->discovery_client);
+
+  name = g_strdup_printf ("%s.%s.local", dnssd_name, _self->name);

This looks wrong: I think you've got the order of the two arguments wrong?

(In reply to comment #1)
> This can be G_DEFINE_INTERFACE() since 2.24 I think.

Done.

(In reply to comment #2)
> +  g_ptr_array_foreach (tmp, add_to_array, managers);
> +  g_ptr_array_free (tmp, FALSE);
> 
> +      g_ptr_array_foreach (managers, copy_to_other_array, out);
> +      g_ptr_array_free (managers, TRUE);
> 
> Same review comment as for the corresponding code in Gabble.

Actually having some tp_g_ptr_array_extend or something wouldn't help because the typo is outside said common code, unless it was "take the pointer array and free it?". Anyway, I fixed the wrong one.

> Really, you should say that the caller takes ownership of the array and of the
> references to channel managers within it, and maybe refer to
> TpBaseConnectionCreateChannelManagersImpl which has the same semantics?

Okay, done.

> This looks wrong: I think you've got the order of the two arguments wrong?

Good catch, fixed.

All the above fixed and pushed to my branch.

Ship it. As we discussed IRL, we'll probably want to re-add support for sidecars, but that can happen later.

http://fuckyeahnouns.com/salut%20plugins