On 09/26/2013 07:22 AM, Ludwig Krispenz wrote:
Hi,
I published the latest version of the patch:
https://fedorahosted.org/389/attachment/ticket/47388/0001-Ticket-47388-RFE-to-imnplement-RFC4533.patch
It fixes the issues raised with the previous versions and I also did
check memory leaks with valgrind, I think the changes to the core
server are minimal, all the stuff is in the plugin, so even if there
is a need for more tests, especially stress tests, I would like to
commit to master and then handle new issues found as bug fixes to the
feature.
In general, it looks good. We should add a plug-in dependency on the
retro changelog plug-in in the dse.ldif template and the update file.
I'm OK if you want to deal with this in another ticket, though it might
just be simple to add into this patch.
Why do we have an empty sync_handle_cnum_result() function?
Now for some real nit-picky stuff... ;)
There's a typo in a comment in sync_persist_initialize(). It has
"persisten" instead of "persistent".
There are a few tab vs. space issues in the definition of SyncRequest in
sync.h. There are also a tab vs. space issues at sync_init.c:111,
sync_init.c:175, sync_persist.c:269, sync_persist.c:665,
sync_util.c:367, and sync_util.c:332.
You should add braces around the body of the if/else statements at
sync_persist.c:369, sync_persist.c:470, sync_persist.c:646,
sync_refresh.c:197, sync_refresh.c:253, sync_refresh.c:337,
sync_refresh.c:356, sync_refresh.c:358, sync_util.c:251,
sync_util.c:544, sync_util.c:608, sync_util.c:682, and result.c:294.
If you address the above issues, I'm OK with you pushing it to the repo.
Thanks,
-NGK
Regards,
Ludwig
On 08/30/2013 05:00 PM, Nathan Kinder wrote:
On 08/30/2013 05:06 AM, Ludwig Krispenz wrote:
Hello,
I completed an implementation of rfc 4533 as a plugin in RHDS,
ticket #47388
https://fedorahosted.org/389/ticket/47388
https://fedorahosted.org/389/attachment/ticket/47388/0001-Implement-RFC-4533-as-plugin-version-1.patch
a document describing the implentation can be found here:
http://port389.org/wiki/Content_synchronization_plugin
Please review the changes and the design doc.
There are still some issues, eg valgrind reports some mem leaks I
still need to fix, but I wanted to get this out and get some feedback.
The plugin code itself could be committed and further improved
without doing harm, but there are a few changes affecting the core
server, so pleas look at least for them.
I'm still reviewing the new code, but I believe that you accidentally
removed the whoami plug-in from Makefile.am.
Thanks,
-NGK
Thanks,
Ludwig
--
389-devel mailing list
389-devel@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/389-devel
--
389-devel mailing list
389-devel@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/389-devel
