Review Request 111650: Adjust RunnerManager to new ThreadWeaver API using JobPointer

Kevin Ottens ervin at kde.org
Wed Jul 24 07:40:24 UTC 2013


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


Looks good overall, I still think a typedef for QSharedPointer<FindMatchesJob> would be worth it.

I'm not giving the "ship it" because I'd like someone who knows RunnerManager internals better than me to review it as it clearly has an impact on the job memory management. Otherwise I'd approve it right away.

- Kevin Ottens


On July 23, 2013, 11:23 a.m., Martin Gräßlin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111650/
> -----------------------------------------------------------
> 
> (Updated July 23, 2013, 11:23 a.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Description
> -------
> 
> ThreadWeaver does not use Job* anymore which made the code compile fail and some connects being wrong.
> 
> This patch adjusts RunnerManager to also use QSharedPointer<FindMatchJob> instead of FindMatchJob*. It
> simplifies a few things as we do no longer have to call e.g. qDeleteAll and fixes the incorrect connects.
> 
> The patch also enables C++11 as it uses auto.
> 
> 
> Diffs
> -----
> 
>   src/plasma/private/runnerjobs.cpp a012a6c 
>   src/plasma/private/runnerjobs_p.h cf05324 
>   src/plasma/runnermanager.h ebeb029 
>   src/plasma/runnermanager.cpp 46857f4 
> 
> Diff: http://git.reviewboard.kde.org/r/111650/diff/
> 
> 
> Testing
> -------
> 
> compiles, but couldn't run the unit tests (something is broken in my setup)
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20130724/d8a98baf/attachment.html>


More information about the Plasma-devel mailing list