[389-devel] Please review: #47388: [RFE] Support 'Content Synchronization Operation' (SyncRepl)

[Nathan Kinder]

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 at lists.fedoraproject.org
>>> https://admin.fedoraproject.org/mailman/listinfo/389-devel
>>
>