Re: [Opensync-devel] [RFC][PATCH] External command in plugin configuration file

[Daniel G. <go...@b1...>]

On Saturday 31 October 2009 11:01:33 am Henrik /KaarPoSoft wrote:
> Following Daniel's checkin of the client and example code for external 
> plugin, here is a patch (on -r5895) which allows a member to set an 
> ExternalCommand in the plugin configuration file. This command will be 
> executed by the proxy to start up the external process.
> 
> Your comments would be most appreciated.
> 

One questions regarding the API:

osync_member_get_external_command()

Is there any reason to export this function to the public API? Our goal should 
be to have only a small number of public API interfaces .. so it's easier to 
maintain and to keep the API stable .. currently i don't see any reason to put 
this inside opensync_member.h

Instead i would suggest to move this into osync_member_internals.h and use 
OSYNC_TEST_EXPORT instead .. not OSYNC_EXPORT. So this function is still 
avaibale for the osync_engine.c code and for unittests...

Beside this the patch looks pretty good to me ...

JFYI, we have a unit test to avoid API breaks - if you add new public 
Interfaces you need to modifiy following file: trunk/opensync.sym

Just add by hand the function name - sorted. And test with:
ctest -R symbol -V

Currently it fails with your patch, due to missing entries:


-----8<----
332a333
> osync_member_get_external_command <------------- IGNORE this!
513a515
> osync_plugin_config_get_externalplugin
522a525
> osync_plugin_config_set_externalplugin
568a572,576
> osync_plugin_externalplugin_get_external_command
> osync_plugin_externalplugin_new
> osync_plugin_externalplugin_ref
> osync_plugin_externalplugin_set_external_command
> osync_plugin_externalplugin_unref
-- Process completed
***Failed 

0% tests passed, 1 tests failed out of 1

The following tests FAILED:
          1 - symbols (Failed)
------->8----

So you need to add follwing lines to opensync.sym:

osync_plugin_config_get_externalplugin
osync_plugin_config_set_externalplugin

osync_plugin_externalplugin_get_external_command
osync_plugin_externalplugin_new
osync_plugin_externalplugin_ref
osync_plugin_externalplugin_set_external_command
osync_plugin_externalplugin_unref


Please move osync_member_get_external_command() to opensync_member_internals.h 
and adapt opensync.sym commit your modifications.

Thanks for your contribution!

Best Regards,
Daniel

-- 
Daniel Gollub                        Geschaeftsfuehrer: Ralph Dehner
FOSS Developer                       Unternehmenssitz:  Vohburg
B1 Systems GmbH                      Amtsgericht:       Ingolstadt
Mobil: +49-(0)-160 47 73 970         Handelsregister:   HRB 3537
EMail: go...@b1...          http://www.b1-systems.de

Adresse: B1 Systems GmbH, Osterfeldstraße 7, 85088 Vohburg
http://pgpkeys.pca.dfn.de/pks/lookup?op=get&search=0xED14B95C2F8CA78D