Bug 27047

Summary:	Add more PubSub functionality, and add hooks for extensibility
Product:	Wocky	Reporter:	Will Thompson <will>
Component:	General	Assignee:	Guillaume Desmottes <guillaume.desmottes>
Status:	RESOLVED FIXED	QA Contact:
Severity:	normal
Priority:	medium	Keywords:	patch
Version:	unspecified
Hardware:	Other
OS:	All
URL:	http://git.collabora.co.uk/?p=user/wjt/wocky.git;a=shortlog;h=refs/heads/moar-pubsub
Whiteboard:
i915 platform:		i915 features:

Description Will Thompson 2010-03-12 04:43:31 UTC

• Support subscribing and unsubscribing from nodes, and listing subscriptions on a service;
• Add a mechanism to listen to all events for any node on a service;
• Allow WockyPubsubService subclasses to produce subclasses of WockyPubsubNode;
• Expose building stanzas for various actions to make it easier to embrace and extend PubSub.

Comment 1 Guillaume Desmottes 2010-03-12 06:44:20 UTC

Looks pretty good to me. Nice code, nice design and well tested. Some minor comments:

+typedef struct {
+    WockyPubsubNode *node;
+    gchar *jid;
+    WockyPubsubSubscriptionState state;
+    gchar *subid;
+} WockyPubsubSubscription;

I'd add a comment explaining, at least, the jid and subid members are it's not ulra obivous if you're not a pubsub guru.

gboolean wocky_pubsub_service_retrieve_subscriptions_finish (
'gboolean' should be on its own line.

http://git.collabora.co.uk/?p=user/wjt/wocky.git;a=commitdiff;h=1553b3db8841fb5b70b8bd1ab7687e06b2dc4e47
I'd add a comment in wocky-pubsub-service.h explaining node_object_type

Comment 2 Will Thompson 2010-03-13 04:48:06 UTC

(In reply to comment #1)
> +typedef struct {
> +    WockyPubsubNode *node;
> +    gchar *jid;
> +    WockyPubsubSubscriptionState state;
> +    gchar *subid;
> +} WockyPubsubSubscription;
> 
> I'd add a comment explaining, at least, the jid and subid members are it's not
> ulra obivous if you're not a pubsub guru.

True. http://git.collabora.co.uk/?p=user/wjt/wocky.git;a=commitdiff;h=28ef5c1 documents it, and references the relevant bits of XEP-0060.

> gboolean wocky_pubsub_service_retrieve_subscriptions_finish (
> 'gboolean' should be on its own line.

http://git.collabora.co.uk/?p=user/wjt/wocky.git;a=commitdiff;h=582b5bb

> http://git.collabora.co.uk/?p=user/wjt/wocky.git;a=commitdiff;h=1553b3db8841fb5b70b8bd1ab7687e06b2dc4e47
> I'd add a comment in wocky-pubsub-service.h explaining node_object_type

http://git.collabora.co.uk/?p=user/wjt/wocky.git;a=commitdiff;h=2938044

In two of the above patches I added the gtkdoc to the .c; IIRC this is the done thing to avoid rebuilding every file in the source tree when you tweak docs.

Comment 3 Guillaume Desmottes 2010-03-15 01:06:06 UTC

Looks good to me.

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.