[Kde-pim] Review Request: akonadi-nepomuk feeder

[Vishesh Handa]

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


I never completed my review. I did test it out though. Feel free to address the issues I've raised whenever, all of them are very minor.


agents/nepomukfeeder/findunindexeditemsjob.cpp
<http://git.reviewboard.kde.org/r/107989/#comment18424>

    Magic number? Why 3?



agents/nepomukfeeder/findunindexeditemsjob.cpp
<http://git.reviewboard.kde.org/r/107989/#comment18421>

    If you want a small performance + readability upgrade you can directly use aneo:akonadiindexCompatLevel and aneo:akonadiItemId in the query instead of Soprano::Node::resourceToN3( .. ).
    
    The resourceToN3 method is faster in 4.9, but from 4.10 it's better to directly write it in the string.



agents/nepomukfeeder/itemqueue.h
<http://git.reviewboard.kde.org/r/107989/#comment18425>

    I'm not sure what the PIM policy is, but in general it's nicer to have proper class documentation.
    
    I should get a general idea of what the class does and why it should be used by just reading this documentation.
    
    Maybe you could improve one this later? It would help other people (specially me) with understanding the code.



agents/nepomukfeeder/itemqueue.cpp
<http://git.reviewboard.kde.org/r/107989/#comment18426>

    Please add #include <Nepomuk2/Vocabulary/NCO> and use NCO::EmailAddress() instead of these long strings.



agents/nepomukfeeder/test/cachetest.cpp
<http://git.reviewboard.kde.org/r/107989/#comment18428>

    Please use NCO::EmailAddress


- Vishesh Handa


On Dec. 28, 2012, 8:01 p.m., Christian Mollekopf wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107989/
> -----------------------------------------------------------
> 
> (Updated Dec. 28, 2012, 8:01 p.m.)
> 
> 
> Review request for KDEPIM, Volker Krause, Vishesh Handa, David Faure, and Laurent Montel.
> 
> 
> Description
> -------
> 
> This changeset adds various performance improvements to the feeders including a cache for nepomuk statements, and changes the initial indexing approach to be a recurring query. This allows to skip incoming notifications for mass-updates which helps to prevent the feeders and changerecorder from getting overloaded.
> 
> One concern left is the performance of the recurring queries, but it works very well for me.
> 
> 
> Diffs
> -----
> 
>   agents/nepomukfeeder/CMakeLists.txt 4fc7dcf85abca7309a550fc85dec119af1fd9d95 
>   agents/nepomukfeeder/feederqueue.h 8c23a7369495dd7aa8ae849adc03ed625c62a3ba 
>   agents/nepomukfeeder/feederqueue.cpp 88e5d48d67b717b8381dd1fa3f1b56443175f908 
>   agents/nepomukfeeder/findunindexeditemsjob.h PRE-CREATION 
>   agents/nepomukfeeder/findunindexeditemsjob.cpp PRE-CREATION 
>   agents/nepomukfeeder/itemqueue.h PRE-CREATION 
>   agents/nepomukfeeder/itemqueue.cpp PRE-CREATION 
>   agents/nepomukfeeder/nepomukfeederagent.h 9900e8296447fcad45c8782ddf3fd3c4b62d2921 
>   agents/nepomukfeeder/nepomukfeederagent.cpp 5425bc2a60f16a529074b30542030782f6732e33 
>   agents/nepomukfeeder/nepomukhelpers.h f9adbf46d2e322542aa9f5e3f5d1c31432257d5b 
>   agents/nepomukfeeder/nepomukhelpers.cpp 8d1e1f0de09f54779d26a112950e3b73c1d55e97 
>   agents/nepomukfeeder/plugins/nepomukcontactfeeder.cpp 9b13af759ea0b8dfc49a22045ca97fab4c1d805a 
>   agents/nepomukfeeder/propertycache.h PRE-CREATION 
>   agents/nepomukfeeder/propertycache.cpp PRE-CREATION 
>   agents/nepomukfeeder/test/CMakeLists.txt 1a4b1aa139eca61cb370d7e38080da6a7d7a97a1 
>   agents/nepomukfeeder/test/cachetest.cpp PRE-CREATION 
>   agents/nepomukfeeder/test/performancetest.cpp 54f926a77097be7a63065aa0ef57b4a6a714895c 
>   agents/nepomukfeeder/test/querytest.cpp PRE-CREATION 
>   agents/nepomukfeeder/util/CMakeLists.txt d2be9942fa068cf880be06563b661be2992e1ea4 
> 
> Diff: http://git.reviewboard.kde.org/r/107989/diff/
> 
> 
> Testing
> -------
> 
> I'm running it.
> 
> 
> Thanks,
> 
> Christian Mollekopf
> 
>

_______________________________________________
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/