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


Code-wise, it has a few issues with the config saving and restoration, those 
are easily fixed, however.

Otherwise, the problem is that we've frozen Plasma 4.x for more than a year 
already, so we won't accept new features. All new feature development will have 
to be done against Plasma 5.x (in kdeplasma-addons, that would be the 
"frameworks" branch). I don't know about the porting status of the binary 
clock, but it'll need some adjustments in order to make it work, porting the UI 
to QML, for example. Best would be to port your patch to the Plasma 5.x 
version, and given the resulting code's quality is OK, I'd be quite fine with 
merging that.


applets/binary-clock/binaryclock.cpp
<https://git.reviewboard.kde.org/r/122196/#comment51688>

    You probably want to read the value of perFieldBinary and initialize the 
checkbox with it. Otherwise it'll always default to false, no matter the actual 
setting.



applets/binary-clock/binaryclock.cpp
<https://git.reviewboard.kde.org/r/122196/#comment51689>

    I think you're forgetting to actually save the config value. Look at the 
cg.writeEntry(...) calls below.


- Sebastian Kügler


On Jan. 22, 2015, 6:47 a.m., Patrick Uiterwijk wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122196/
> -----------------------------------------------------------
> 
> (Updated Jan. 22, 2015, 6:47 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: kdeplasma-addons
> 
> 
> Description
> -------
> 
> Offer the option to show per-field binary instead of per-digit
> 
> 
> Diffs
> -----
> 
>   applets/binary-clock/binaryclock.h 5ae83dfff9c473bd484b99d49330093b6e091975 
>   applets/binary-clock/binaryclock.cpp 
> 3b8c7af4631fcb60437045eae37d0c0552c85299 
>   applets/binary-clock/clockConfig.ui 
> 0c2cec7925dd9e1ff92d7cd9d99005dca20497b0 
> 
> Diff: https://git.reviewboard.kde.org/r/122196/diff/
> 
> 
> Testing
> -------
> 
> I have compiled a patched version of kdeplasma-addons on a recent version of 
> Fedora (22), and am using it myself.
> 
> 
> Thanks,
> 
> Patrick Uiterwijk
> 
>

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to