Review Request: Port t-i-d to Nepomuk::Query + bonus fixes

[Sebastian Trueg]

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

Ship it!


I am always happy to review and Nepomuk-related patches. I only have two comments and those are only related to the query API changes we made. From what I can see you made perfect use of all Nepomuk parts.


/trunk/playground/network/telepathy-integration-daemon/telepathyaccount.cpp
<http://reviewboard.kde.org/r/3936/#comment5233>

    You know what? Why didn't we make the QueryServiceClient::syncXXX methods static? IMHO that would fit perfectly.



/trunk/playground/network/telepathy-integration-daemon/telepathycontact.cpp
<http://reviewboard.kde.org/r/3936/#comment5234>

    Again not really related to the review request: do you think it is good to have Soprano::Node for the additional bindings or should it rather be Nepomuk::Variant?


- Sebastian


On 2010-05-09 16:33:56, Dario Freddi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3936/
> -----------------------------------------------------------
> 
> (Updated 2010-05-09 16:33:56)
> 
> 
> Review request for telepathy, Sebastian Trueg and George Goldberg.
> 
> 
> Summary
> -------
> 
> This patch ports t-i-d to Nepomuk::Query from raw SPARQL calls. It also increases the dependency to KDE 4.5 to take advantage of all the fixes+features to N::Q which were necessary for this patch.
> 
> I also took the chance to improve the routines a bit: now literal matches (like IDs and paths) are done in the query itself, which should also improve performance a bit. Also, I refactored the query for finding out contacts for a specific account: now all contacts are retrieved (isBuddyOf is a dynamic property, including it in the query as a requirement lead to creating the resources for accounts which are no longer our buddies every time t-i-d started), and a special case is added for contacts which belong to ourself.
> 
> This might happen when I am a buddy of a contact I own: in this case, we definitely want a separate PersonContact+IMAccount to avoid race conditions.
> 
> Sebastian, I included you in the reviewers so you can have a look to how I used the new service :) I'll include you in the next one as well, which is way more complex as well.
> 
> 
> Diffs
> -----
> 
>   /trunk/playground/network/telepathy-integration-daemon/CMakeLists.txt 1124586 
>   /trunk/playground/network/telepathy-integration-daemon/telepathyaccount.h 1124586 
>   /trunk/playground/network/telepathy-integration-daemon/telepathyaccount.cpp 1124586 
>   /trunk/playground/network/telepathy-integration-daemon/telepathycontact.cpp 1124586 
> 
> Diff: http://reviewboard.kde.org/r/3936/diff
> 
> 
> Testing
> -------
> 
> All fine, no regressions and some fixes instead.
> 
> 
> Thanks,
> 
> Dario
> 
>