Review Request 121365: Update brightness availability at runtime

Sebastian Kügler sebas at kde.org
Mon Dec 8 14:09:13 UTC 2014


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

Ship it!


It *looks* sane to me, but I'm not 100% comfortable since I don't know the code by heart. Let's see if within a few days someone else comes up who can look over this patch and is more qualified than I am. Otherwise, I'd suggest to push it in *ANY* case before our beta, so we can get wider end-user feedback. I know we have bugs open about brightness, and it *looks* like they might be fixed with these patches, so I'm all for improving the code.


dataengines/powermanagement/powermanagementengine.cpp
<https://git.reviewboard.kde.org/r/121365/#comment49906>

    I suppose this change comes from powerdevil itself? (Please check, you probably know why you did that change, but it's not immediately obvious to me, if you know it's correct, no further action needed of course.)



dataengines/powermanagement/powermanagementengine.cpp
<https://git.reviewboard.kde.org/r/121365/#comment49904>

    if you're changing these lines anyway, you may as well use QStringLiteral for these.



dataengines/powermanagement/powermanagementengine.cpp
<https://git.reviewboard.kde.org/r/121365/#comment49905>

    can we use compile-time connections here? We recently saw this kind of code breaking (especially likely since we're tapping into 3rd party dbus APIs here)


It

- Sebastian Kügler


On Dec. 6, 2014, 12:48 a.m., Kai Uwe Broulik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121365/
> -----------------------------------------------------------
> 
> (Updated Dec. 6, 2014, 12:48 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> -------
> 
> This listens for maximum brightness changes to update the availability accordingly. It does not, however, monitor for the brightnesscontrols interface to become available on dbus and I have no idea how that would be done.
> 
> 
> Diffs
> -----
> 
>   dataengines/powermanagement/powermanagementengine.h 8088209 
>   dataengines/powermanagement/powermanagementengine.cpp a9020cf 
> 
> Diff: https://git.reviewboard.kde.org/r/121365/diff/
> 
> 
> Testing
> -------
> 
> Works as before, cannot really test that since I cannot rip out my keyboard. Please see the other review.
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

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


More information about the Plasma-devel mailing list