Re: [Kde-hardware-devel] Review Request 121815: Cleanup BrightnessOSDWidget

[Kai Uwe Broulik]

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

(Updated Jan. 4, 2015, 12:01 p.m.)


Status
--

This change has been marked as submitted.


Review request for Solid.


Repository: powerdevil


Description
---

It's no longer that fullblown widget it used to be bust just calls Plasma's OSD 
service via DBus. It might as well just be a namespace with a function inside, 
doesn't need to be a class.


Diffs
-

  daemon/actions/bundled/brightnesscontrol.h 1c6f343 
  daemon/actions/bundled/brightnesscontrol.cpp 27355a0 
  daemon/actions/bundled/keyboardbrightnesscontrol.h 4dbc4cc 
  daemon/actions/bundled/keyboardbrightnesscontrol.cpp de4c0a4 
  daemon/brightnessosdwidget.h 828b242 
  daemon/brightnessosdwidget.cpp f85e25c 

Diff: https://git.reviewboard.kde.org/r/121815/diff/


Testing
---

Works like it did before.


Thanks,

Kai Uwe Broulik

___
Kde-hardware-devel mailing list
Kde-hardware-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-hardware-devel

Re: [Kde-hardware-devel] Review Request 121798: KScreen fade effect connector for PowerDevil

[Martin Gräßlin]

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


Concerning locking on non-Logind: the code should be removed from powerdevil (I 
think I have a review request open for that). Nevertheless I would like to 
still have the feature available in screen locker.


daemon/kwineffect.h


I don't like the name: it's too generic. What's *the* KWinEffect



daemon/kwineffect.cpp


I think type should be XCB_ATOM_CARDINAL and lenght only needs to be 1



daemon/kwineffect.cpp


I'd suggest to also verify that format is 32



daemon/kwineffect.cpp


the type xcb_atom_t looks wrong to me. It should just be a uint32_t IIRC


- Martin Gräßlin


On Jan. 3, 2015, 1:14 p.m., Kai Uwe Broulik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121798/
> ---
> 
> (Updated Jan. 3, 2015, 1:14 p.m.)
> 
> 
> Review request for Solid, Àlex Fiestas and Martin Gräßlin.
> 
> 
> Bugs: 340063
> https://bugs.kde.org/show_bug.cgi?id=340063
> 
> 
> Repository: powerdevil
> 
> 
> Description
> ---
> 
> This is an implementation of the client side of the KScreen effect for 
> PowerDevil eventually allowing it to fade the screen before going to sleep. 
> The code is loosely based on Alex' initial work [1] but ported to XCB.
> 
> [1] 
> http://quickgit.kde.org/?p=libkscreen.git&a=commit&h=70cf4482ed2e83d14c2ec9b805921ba4eb741082
> 
> 
> Diffs
> -
> 
>   daemon/CMakeLists.txt f26f786 
>   daemon/config-powerdevil.h.cmake b730cd9 
>   daemon/kwineffect.h PRE-CREATION 
>   daemon/kwineffect.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/121798/diff/
> 
> 
> Testing
> ---
> 
> Calling start() causes the scren to dim, the state is properly updated by 
> KWin and the respective signals are fired.
> 
> As for the implementation in the suspendsession action I have to agree with 
> Martin here that we should drop the support for locking on non-logind 
> platforms since it makes the code utterly complex and isn't secure anyway. 
> That way PowerDevil could just kick off the fade effect, wait for it to be 
> done and then tell Logind to suspend which completely takes care of the 
> locking (and more importantly the waiting for the lock to be established 
> *before* powering down).
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

___
Kde-hardware-devel mailing list
Kde-hardware-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-hardware-devel