[Kde-hardware-devel] Review Request 109352: Add hybrid suspend method to kde-workspace

[Dario Freddi]

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


Overall, the patch looks fine. What I don't like is the huge mess of names we have among variables/methods/UI strings between Sleep/Suspend/etc... . To be honest, I'd like to see with this patch a unification of everything, where we decide how do we call what (Hibernate included) and be consistent among all of the code and the UI. I admit for a newcomer to the codebase all of this starts to be really really confusing, and exposing different messages in the UI is really bad.


plasma/desktop/applets/kickoff/core/leavemodel.cpp
<http://git.reviewboard.kde.org/r/109352/#comment31891>

    I see no #endif here... am I missing something? (same below)



plasma/generic/runners/powerdevil/PowerDevilRunner.cpp
<http://git.reviewboard.kde.org/r/109352/#comment31892>

    Whitespace



powerdevil/daemon/powerdevilfdoconnector.cpp
<http://git.reviewboard.kde.org/r/109352/#comment31893>

    Watch out: this interface is meant for compatibility with fdo ( not that anyone uses it anymore, though... ) and so it should stay as it is. Although I think at some point we might just as well drop it, as we're the only ones to implement it I think.


- Dario Freddi


On Nov. 27, 2013, 6:04 p.m., Lukáš Tinkl wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109352/
> -----------------------------------------------------------
> 
> (Updated Nov. 27, 2013, 6:04 p.m.)
> 
> 
> Review request for Solid, Àlex Fiestas and Dario Freddi.
> 
> 
> Bugs: 302968
>     http://bugs.kde.org/show_bug.cgi?id=302968
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> -------
> 
> This patch adds the implementation of hybrid suspend method, complementing https://git.reviewboard.kde.org/r/109351/.
> 
> The hybrid suspend approach has the advantages of regular suspend (faster resuming than with hibernation) while being safer at the same time in case the battery runs out. More info: http://en.wikipedia.org/wiki/Sleep_mode#Hybrid_sleep
> 
> Affected components: shutdown dialog (ksmserver), kickoff, battery monitor, lock_logout applet, powermanagement data engine, powerdevil & its runner
> 
> Hybrid suspend becomes the preferred method to put the system into sleep thus greatly simplifying the user interface; if it's not available (because of system or kernel limitation), the current two options (suspend + hibernate) will be presented to the user. 
> 
> Technically, powerdevil has supported this approach since the beginning and both HAL and upower/systemd backends already implement it.
> 
> 
> Diffs
> -----
> 
>   ksmserver/shutdowndlg.cpp 247c877 
>   ksmserver/themes/default/main.qml 8924fac 
>   plasma/desktop/applets/kickoff/core/itemhandlers.cpp 4e83c37 
>   plasma/desktop/applets/kickoff/core/leavemodel.cpp 47559ce 
>   plasma/desktop/applets/kickoff/simpleapplet/simpleapplet.cpp 28fba18 
>   plasma/generic/applets/batterymonitor/contents/code/logic.js 974694a 
>   plasma/generic/applets/batterymonitor/contents/code/platform.js 59af3af 
>   plasma/generic/applets/batterymonitor/contents/ui/PopupDialog.qml 76fb1ce 
>   plasma/generic/applets/batterymonitor/platformcontents/touch/code/platform.js aa57ab8 
>   plasma/generic/applets/lock_logout/contents/ui/config.ui 07579b4 
>   plasma/generic/applets/lock_logout/contents/ui/data.js 9608a21 
>   plasma/generic/dataengines/powermanagement/powermanagementengine.h 512bf6e 
>   plasma/generic/dataengines/powermanagement/powermanagementengine.cpp 33a6e03 
>   plasma/generic/dataengines/powermanagement/powermanagementjob.h 3b974ab 
>   plasma/generic/dataengines/powermanagement/powermanagementjob.cpp 1b0b8a0 
>   plasma/generic/runners/powerdevil/PowerDevilRunner.cpp 74eb36d 
>   powerdevil/daemon/org.kde.Solid.PowerManagement.xml 70e2c65 
>   powerdevil/daemon/powerdevilbackendinterface.h 91519a9 
>   powerdevil/daemon/powerdevilcore.h 77c3f2f 
>   powerdevil/daemon/powerdevilcore.cpp 2dcdbc6 
>   powerdevil/daemon/powerdevilfdoconnector.h 0739187 
>   powerdevil/daemon/powerdevilfdoconnector.cpp 9fd2360 
> 
> Diff: http://git.reviewboard.kde.org/r/109352/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Lukáš Tinkl
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-hardware-devel/attachments/20131128/df9a9306/attachment-0001.html>