[Bug 44649] Gabble plugin API symbols should be factored out to a separate library

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Mon Jan 23 20:33:56 CET 2012


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

--- Comment #13 from Jonny Lamb <jonny.lamb at collabora.co.uk> 2012-01-23 11:33:56 PST ---
This seems fair enough but there's a lot of style errors. Perhaps you might
want to consult this wiki page:

http://telepathy.freedesktop.org/wiki/Style

- * connection.h - connection API available to telepathy-gabble plugins
- * Copyright © 2010 Collabora Ltd.
- * Copyright © 2010 Nokia Corporation
...
+ * plugin-connection.h — Connection API available to telepathy-gabble plugins
+ * Copyright © 2009 Collabora Ltd.
+ * Copyright © 2009 Nokia Corporation

How did this change from 2010 to 2009? Also, you should update the year for
Collabora. Same thing for the source file.

+ GabblePluginConnection *plugin_connecion,

connection

-     TpBaseConnection *connection)
+    GabblePluginConnection *plugin_connection,
+    TpBaseConnection *connection)

-  tmp = gabble_plugin_loader_create_channel_managers (loader, conn);
+   tmp = gabble_plugin_loader_create_channel_managers (loader,
plugin_connection,

Why the change in indentation in these?

+lib_LTLIBRARIES = libgabble-plugin.la

Can I be really annoying and ask you to change this to -plugins instead of
-plugin please?

+     G_IMPLEMENT_INTERFACE (GABBLE_TYPE_PLUGIN_CONNECTION,
+       gabble_connection_plugin_service_iface_init);

Seems an odd iface_init function name choice?

+gchar *
+_gabble_plugin_connection_get_full_jid (GabblePluginConnection *plugin_conn)

Should be static, no? Same with most of the other functions that are set in
GabblePluginConnectionIface the struct as they're not used anywhere else in
gabble?

+ sig_id_porter_available = g_signal_new (
+ "porter-available",
+ G_OBJECT_CLASS_TYPE (iface),
+ G_SIGNAL_RUN_LAST | G_SIGNAL_DETAILED, 0, NULL, NULL,
+ g_cclosure_marshal_VOID__OBJECT, G_TYPE_NONE, 1, WOCKY_TYPE_PORTER);
+ g_once_init_leave (&once, 1);

Indentation please.

+ * SECTION: gabble-connection-plugin-service

Seems this needs to be updated.

+GType
+gabble_plugin_connection_get_type (void)

Could you use G_DEFINE_INTERFACE() here instead, to save some lines?

+gabble_plugin_connection_get_caps (
+    GabblePluginConnection *plugin_connection, TpHandle handle)

One argument per line please.

There's also some random whitespace added here and there out of context. You
should review your own patch before submitting it here.

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