----------------------------------------------------------- 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 > >
_______________________________________________ Plasma-devel mailing list [email protected] https://mail.kde.org/mailman/listinfo/plasma-devel
