Review Request 121581: Plotter in kdeclarative

[David Edmundson]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/121581/#review73836
-----------------------------------------------------------

Ship it!


Looks good, I've been playing with it and this final patch addresses all my comments.

Personally I'd use QMutexLocker more, it would simplify some code and you don't need to worry about early returns, but all your current stuff looks good. Add a comment on why it's needed, and one next to ::render to say it's called from another thread.

- David Edmundson


On Dec. 29, 2014, 12:17 p.m., Marco Martin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121581/
> -----------------------------------------------------------
> 
> (Updated Dec. 29, 2014, 12:17 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: kdeclarative
> 
> 
> Description
> -------
> 
> This is an alternative, mutually exclusive to
> https://gerrit.vesnicky.cesnet.cz/r/#/c/244
> and dependent from
> https://git.reviewboard.kde.org/r/121575/
> 
> since the plotter component doesn't depend from libplasma, it may be useful to have it outside of libplasma, so any applciation that wants it may use it.
> Any opinion whether this should go here or in libplasma is welcome.
> 
> 
> Diffs
> -----
> 
>   src/qmlcontrols/kquickcontrolsaddons/kquickcontrolsaddonsplugin.cpp 0e2eb2f 
>   src/qmlcontrols/kquickcontrolsaddons/plotter.h PRE-CREATION 
>   src/qmlcontrols/kquickcontrolsaddons/plotter.cpp PRE-CREATION 
>   CMakeLists.txt 233ccce 
>   cmake/Findepoxy.cmake PRE-CREATION 
>   src/qmlcontrols/kquickcontrolsaddons/CMakeLists.txt 786aaa5 
> 
> Diff: https://git.reviewboard.kde.org/r/121581/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

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