[Kde-pim] Review Request 111149: Add file monitoring to ICalDir and VCardDir resources and move common code to shared base class

[Kevin Krammer]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111149/#review34760
-----------------------------------------------------------


One general remark: I think it would be better to use different identifiers for the files than the UIDs of the entities. The UID could contain characters that are not allowed in the filesystem. I think there is even a bug report for that.


resources/shared/dirresource.h
<http://git.reviewboard.kde.org/r/111149/#comment25482>

    & at the argument name, not at the type



resources/shared/dirresource.h
<http://git.reviewboard.kde.org/r/111149/#comment25483>

    position of &



resources/shared/dirresource.h
<http://git.reviewboard.kde.org/r/111149/#comment25481>

    newItem is missing the MIME type



resources/shared/dirresource.h
<http://git.reviewboard.kde.org/r/111149/#comment25484>

    position of &



resources/shared/dirresource.h
<http://git.reviewboard.kde.org/r/111149/#comment25485>

    position of &



resources/shared/dirresource.h
<http://git.reviewboard.kde.org/r/111149/#comment25486>

    position of &



resources/shared/dirresource.h
<http://git.reviewboard.kde.org/r/111149/#comment25487>

    position of &



resources/shared/dirresource.h
<http://git.reviewboard.kde.org/r/111149/#comment25488>

    position of &



resources/shared/dirresource.h
<http://git.reviewboard.kde.org/r/111149/#comment25489>

    position of &



resources/shared/dirresource.h
<http://git.reviewboard.kde.org/r/111149/#comment25490>

    position of &



resources/shared/dirresource.h
<http://git.reviewboard.kde.org/r/111149/#comment25491>

    Hmm, the resource should be able to deleted an item by remoteId



resources/shared/dirresource.h
<http://git.reviewboard.kde.org/r/111149/#comment25492>

    I am not so sure it is necessary to load all the data into memory.
    



resources/shared/dirresourcebase.cpp
<http://git.reviewboard.kde.org/r/111149/#comment25493>

    that's not needed anymore, right?



resources/shared/dirresourcebase.cpp
<http://git.reviewboard.kde.org/r/111149/#comment25494>

    position of &



resources/shared/dirresourcebase.cpp
<http://git.reviewboard.kde.org/r/111149/#comment25495>

    position of *



resources/shared/dirresourcebase.cpp
<http://git.reviewboard.kde.org/r/111149/#comment25496>

    since this is an enum, wouldn't a switch() be cleaner?


- Kevin Krammer


On June 20, 2013, 9:24 a.m., Dan Vrátil wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111149/
> -----------------------------------------------------------
> 
> (Updated June 20, 2013, 9:24 a.m.)
> 
> 
> Review request for KDEPIM.
> 
> 
> Description
> -------
> 
> Similarily to iCal and vCard resources, the iCalDir and vCardDir resources are essentially identical. This patch moves all the code to a template-based DirResource class, making VCardDirResource and ICalDirResource specializations of the DirResource class, implementing only loading and saving of payload from/to file. QObject-related stuff is implemented in DirResourceBase class.
> 
> The shared code now also supports watching the directory (via KDirWatch) for changes and will automatically add, update or remove entries from Akonadi when a file is added, changed or removed in the resource's data directory.
> 
> 
> This addresses bug 321343.
>     http://bugs.kde.org/show_bug.cgi?id=321343
> 
> 
> Diffs
> -----
> 
>   resources/icaldir/CMakeLists.txt 3ca6c5e 
>   resources/icaldir/icaldirresource.h 8fe7733 
>   resources/icaldir/icaldirresource.cpp b294be0 
>   resources/shared/dirresource.h PRE-CREATION 
>   resources/shared/dirresourcebase.h PRE-CREATION 
>   resources/shared/dirresourcebase.cpp PRE-CREATION 
>   resources/vcarddir/CMakeLists.txt 7a58e6f 
>   resources/vcarddir/vcarddirresource.h 1423bdf 
>   resources/vcarddir/vcarddirresource.cpp 5d17d26 
> 
> Diff: http://git.reviewboard.kde.org/r/111149/diff/
> 
> 
> Testing
> -------
> 
> Set up vCardDir resource to watch for /tmp/vcarddir. Moved a new .vcard file into the folder and it was immediately added to Akonadi. Changing the file updated payload stored in Akonadi and removing the file removed respective item from Akonadi. Same for the iCalDir.
> 
> 
> Thanks,
> 
> Dan Vrátil
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/