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

Dario Freddi drf54321 at gmail.com
Sun May 9 20:31:15 CEST 2010



> On 2010-05-09 18:24:25, Sebastian Trueg wrote:
> > /trunk/playground/network/telepathy-integration-daemon/telepathyaccount.cpp, line 137
> > <http://reviewboard.kde.org/r/3936/diff/1/?file=26056#file26056line137>
> >
> >     You know what? Why didn't we make the QueryServiceClient::syncXXX methods static? IMHO that would fit perfectly.

Now, that's a good point. Do you want me to take care of that?


> On 2010-05-09 18:24:25, Sebastian Trueg wrote:
> > /trunk/playground/network/telepathy-integration-daemon/telepathycontact.cpp, line 161
> > <http://reviewboard.kde.org/r/3936/diff/1/?file=26057#file26057line161>
> >
> >     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?

Probably a Variant would be better. From my point of view, one should be able to create a resource (like a IMAccount in this case) just by passing the additionalBinding to the constructor.


- Dario


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


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



More information about the KDE-Telepathy mailing list