[Kde-hardware-devel] Review Request 122048: Don't suspend when closing the lid and an external monitor is connected

Sebastian Kügler sebas at kde.org
Wed Jan 14 13:50:35 UTC 2015



> On Jan. 14, 2015, 12:29 p.m., Sebastian Kügler wrote:
> > daemon/actions/bundled/handlebuttonevents.h, line 68
> > <https://git.reviewboard.kde.org/r/122048/diff/1/?file=341828#file341828line68>
> >
> >     Off-hand, I don't understand what these vars ar e doing by reading their name. Would be nice if it was more clear for the next person to look at this code. Perhaps a comment?
> 
> Kai Uwe Broulik wrote:
>     Right, I didn't want to make the variable name even longer but I'll try to come up with sth.

Life-Pro-Tip: Don't optimize your code for short variable names, optimize for readability. If that means super long names, so be it. Autocompletion is your friend. :)


- Sebastian


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


On Jan. 13, 2015, 11:16 p.m., Kai Uwe Broulik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122048/
> -----------------------------------------------------------
> 
> (Updated Jan. 13, 2015, 11:16 p.m.)
> 
> 
> Review request for Solid, KDE Usability, Àlex Fiestas, and Daniel Vrátil.
> 
> 
> Repository: powerdevil
> 
> 
> Description
> -------
> 
> Less sensational headline: Skip lid action when external monitor is connected.
> 
> This brings the KScreen killer feature in the 4.x times back. Now you can watch movies and safely close the lid again!
> 
> The confusing "Never prevent an action on lid close" is also moved to the main page since it only affects the lid action and is used nowhere else. I'm not happy with the wording but "inhibition" is a difficult thing to describe for the average user.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 27f162c 
>   PowerDevilSettings.kcfg 1bc7ce1 
>   daemon/CMakeLists.txt f1e6efb 
>   daemon/actions/bundled/handlebuttonevents.h 8ea23f6 
>   daemon/actions/bundled/handlebuttonevents.cpp ac280f4 
>   daemon/actions/bundled/handlebuttoneventsconfig.h a55bca7 
>   daemon/actions/bundled/handlebuttoneventsconfig.cpp 92f0cef 
>   kcmodule/global/GeneralPage.cpp 5d9ff10 
>   kcmodule/global/generalPage.ui 26204cb 
> 
> Diff: https://git.reviewboard.kde.org/r/122048/diff/
> 
> 
> Testing
> -------
> 
> Laptop only, monitor option on -> Suspend
> Laptop only, monitor option off -> Suspend
> TV connected, monitor option on -> No action
> TV connected, monitor option off -> Suspend
> 
> PM enabled, inhibit option on -> Suspend
> PM disabled, inhibit option on -> No suspend
> PM enabled, inhibit option off -> Suspend
> PM disabled, inhibit option off -> Suspend
> 
> 
> File Attachments
> ----------------
> 
> Config option
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/01/13/e0956548-0a0f-4c41-b3dd-68a5f17815a5__powerdevilstuff.png
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-hardware-devel/attachments/20150114/5780640e/attachment.html>


More information about the Kde-hardware-devel mailing list