[Bug 45716] salut plugin api needs to be split out and refactored similar to the changes done to gabble

[bugzilla-daemon at freedesktop.org]

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

--- Comment #15 from Olli Salli <olli.salli at collabora.co.uk> 2012-03-14 09:17:53 PDT ---
Here's a full review.

Generally, we'd prefer branches with one commit for each independent change;
that helps understanding what the branch tries to achieve. I know this for this
case in advance though because I planned this work, but for other reviewers,
what this is about:
* The ytstenut plugin creates a custom SalutProtocol in its initialize() method
- but salut_protocol_new recursively depends on loads of salut symbols through
its use of salut_protocol_get_type(), so we changed direct binding to it to a
callback
* Symbols from caps-channel-manager.c were being used in plugins, but it wasn't
in the plugins lib - it now is (it had no additional dependencies)
* the stuff moved from util.c ditto - but it depends on the definition of
SalutContact. I initially observed the dep as just a sanity check but actually
it's functional.

Also, you can include a rationale separately for each change in the commit
message when you have one thing per commit.

Please refactor your commits along those lines.

(In reply to comment #14)
> -      if (gabble_capability_set_has (contact->caps, node))
> -        send_stanza_to_contact (porter, WOCKY_CONTACT (contact), stanza);
> -    }
> 
> +      //if (gabble_capability_set_has (contact->caps, node))
> +        send_stanza_to_contact (porter, WOCKY_CONTACT (contact), stanza);
> +    }
> 
> This behavioral change seems totally unjustified.
> 

So this is because SalutContact includes the caps set, but WockyContact
doesn't.

I discussed this with Alvaro in IRC: we need to implement a trivial
SalutPluginContact GInterface which SalutContact implements. This GInterface
should have a salut_plugin_contact_get_caps() method which allows getting the
caps - similar to the accessors for SalutConnection members in
SalutPluginConnection.

-rw-r--r--    salut/plugin-util.h (renamed from salut/util.h)    3    

Please don't rename headers in the plugin API unnecessarily. This header
doesn't have to have the exact same basename as the .c where it's implemented -
the relationship is clear in any case.

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