Bug 27311

> +WockyXmppStanza *wocky_pubsub_service_create_create_node_stanza (

Rename to w_p_s_new_c_n_s or w_p_s_make_c_n_s? Or keep the current name for comedy value, either works :-)

> +WockyXmppStanza *
> +wocky_pubsub_node_make_delete_stanza (
> +    WockyPubsubNode *self,
> +    WockyXmppNode **pubsub_node,
> +    WockyXmppNode **delete_node)
...
> +  WockyXmppNode *p, *d; /* if we didn't have these, we'd have a great album */
...
> +        WOCKY_NODE_ASSIGN_TO, &p,
...
> +  if (pubsub_node != NULL)
> +    *pubsub_node = p;

Perhaps "WOCKY_NODE_ASSIGN_TO, NULL," should be defined to be a no-op, so that users of stanza_build don't have to do this dance every time?

> -  /* This stanza is inspired from Example 2 of XEP-0004: Data Forms */
> +  /* This stanza is inspired from Example 2 of XEP-0004: Data Form */

-1, over-zealous search and replace. Example 6 has the same problem.

> +data_form_set_value (WockyDataForm *form,
> +    const gchar *field_name,
> +    GValue *value,

Please say explicitly that the value must be slice-allocated.

Other than that, this all looks good.

(In reply to comment #1)
> > +WockyXmppStanza *wocky_pubsub_service_create_create_node_stanza (
> 
> Rename to w_p_s_new_c_n_s or w_p_s_make_c_n_s? Or keep the current name for
> comedy value, either works :-)

I'm pretty keen on the comedy value! Also, I'm increasingly convinced that the proliferation of create_foo_stanza() functions is sub-optimal, particularly when the extra information that needs stamping onto them is pretty uniform. I'm trying to think of better API for it, but haven't figured it out yet...

> > +WockyXmppStanza *
> > +wocky_pubsub_node_make_delete_stanza (
> > +    WockyPubsubNode *self,
> > +    WockyXmppNode **pubsub_node,
> > +    WockyXmppNode **delete_node)
> ...
> > +  WockyXmppNode *p, *d; /* if we didn't have these, we'd have a great album */
> ...
> > +        WOCKY_NODE_ASSIGN_TO, &p,
> ...
> > +  if (pubsub_node != NULL)
> > +    *pubsub_node = p;
> 
> Perhaps "WOCKY_NODE_ASSIGN_TO, NULL," should be defined to be a no-op, so that
> users of stanza_build don't have to do this dance every time?

Yeah, I wondered about this too. It's slightly counter-intuitive though: hey, please assign this node to ... oh actually, don't. But in practice it's probably harmless. I'll make a separate branch for this if that's okay?

> > -  /* This stanza is inspired from Example 2 of XEP-0004: Data Forms */
> > +  /* This stanza is inspired from Example 2 of XEP-0004: Data Form */
> 
> -1, over-zealous search and replace. Example 6 has the same problem.

Fixed.
 
> > +data_form_set_value (WockyDataForm *form,
> > +    const gchar *field_name,
> > +    GValue *value,
> 
> Please say explicitly that the value must be slice-allocated.

Fixed.

Merged on the assumption that my patches were okay. Filed bug 27317 about build.

http://git.collabora.co.uk/?p=wocky.git;a=commit;h=01a69bedb2b77e31c60c9e489e99b625c59b7959