Review Request 113179: Make the runners compile again

[Marco Martin]

On Oct. 8, 2013, 9:15 p.m., Aleix Pol Gonzalez wrote:
> > There's also runner-related code in src/declarativeimports/{core,runnermodel}, these should be reenabled as well. Possibly, they also need changes to make built. I had disabled this after a discussion with Aaron (who has refactored these classes) and Vishesh, who plans to work on them. I'm not sure what's the most useful, I can imagine this leading to a lot of conflicts when merging this refactoring, while the benefits of this patch are then just going away. Aaron can weigh in here to say what's easiest moving forward. Mirko is the best person to review the ThreadWeaver API changes you implement. Could you add him to the reviewers for this request?
> > 
> > thanks...
> 
> Aleix Pol Gonzalez wrote:
>     Well, the only reason I did this was because kickoff in kde-workspace requires runners as well. If those are not good anymore, maybe it's wiser to disable them there as well?
> 
> Sebastian Kügler wrote:
>     Ah, I've missed these. I was operating under the assumption that nobody would use runners in Plasma2 yet -- wrong. We can do both, disable runners in kickoff or merge this. Let's hear others' opinions.
> 
> Kevin Ottens wrote:
>     It overall looks fine to me. We'd need someone with a more intimate knowledge of krunner to look at it though.

if Mirko can say the changes in threadweaver are correct, for me is fine as well, the lest we keep dead code in the repo the better it is


- Marco


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


On Oct. 8, 2013, 5:14 p.m., Aleix Pol Gonzalez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/113179/
> -----------------------------------------------------------
> 
> (Updated Oct. 8, 2013, 5:14 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> -------
> 
> Recently there's been some big changes in the ThreadWeaver API. So big that they break the source compatibility we were supposed to maintain, big time.
> 
> This patch tries to port plasma-framework to that new API.
> 
> 
> Diffs
> -----
> 
>   src/plasma/CMakeLists.txt 411256a 
>   src/plasma/private/runnerjobs.cpp d11f7e9 
>   src/plasma/private/runnerjobs_p.h 7fd7076 
>   src/plasma/private/storage.cpp 35b222e 
>   src/plasma/runnermanager.cpp 135279e 
>   src/plasma/scripting/runnerscript.cpp d5142a2 
> 
> Diff: http://git.reviewboard.kde.org/r/113179/diff/
> 
> 
> Testing
> -------
> 
> Everything builds, including kde-workspace.
> 
> Couldn't test it, there's no tests for it or even an application to test it :/.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

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