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
|
