[Nepomuk] Review Request: QueryService: Decouple the SearchRunnables and the Folder class

[Simeon Bird]

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

Ship it!


Looks good, and makes the code much simpler and more elegant.

I agree that you don't need m_runnableMutex anymore, because the slots will now be executed in the same thread as the folder, instead of the QRunnable's thread, right? So all methods of Folder live in a single-thread, and the mutex doesn't do anything anymore. So yes, I vote for ditching it.

- Simeon Bird


On Jan. 8, 2013, 5:36 p.m., Vishesh Handa wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108271/
> -----------------------------------------------------------
> 
> (Updated Jan. 8, 2013, 5:36 p.m.)
> 
> 
> Review request for Nepomuk, David Faure and Simeon Bird.
> 
> 
> Description
> -------
> 
>     The SearchRunnable classes used to contain a reference to the Folder and
>     call certain functions on it. Since the SearchRunnables were in another
>     thread this required mutexes and still often resulted in deadlocks and
>     crashes.
>     
>     The new code uses signals and slots to communicate between the Folder
>     and the SearchRunnable classes. This is a lot more elegant and easier to
>     understand.
> 
> As far as I see the m_runnableMutex is no longer required. Could someone confirm? Or should I just let it be?
> 
> 
> Diffs
> -----
> 
>   services/storage/query/countqueryrunnable.h 3086656 
>   services/storage/query/countqueryrunnable.cpp 14c7917 
>   services/storage/query/folder.h 4baf092 
>   services/storage/query/folder.cpp 0fed8d2 
>   services/storage/query/searchrunnable.h ca4f596 
>   services/storage/query/searchrunnable.cpp 81a23ac 
> 
> Diff: http://git.reviewboard.kde.org/r/108271/diff/
> 
> 
> Testing
> -------
> 
> Basic functionality testing done. It's a little hard to test the multi-threading part of it.
> 
> 
> Thanks,
> 
> Vishesh Handa
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/nepomuk/attachments/20130109/49d14a94/attachment-0001.html>