Review Request 125802: Split generic parts from X11Locker into AbstractLocker

[David Edmundson]

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

Ship it!


ship it!

I include a grumble about something that existed before this patch, so you can consier it later (or ignore it)


ksmserver/screenlocker/abstractlocker.h (line 68)
<https://git.reviewboard.kde.org/r/125802/#comment60008>

    I don't like this set.
    
    It means if you use globalAccel() from X11Locker's constructor, you crash.
    
    We currently don't, but it's leaving the safety off for the next person who edits this file.
    
    I would either:
     - make a public accessor in KSLApp (which owns this object) and port code to  KSLDApp::self()->globalAccel(), getting rid m_globalAccel here.
    
     - pass it in the constructor to AbstractLocker


- David Edmundson


On Oct. 26, 2015, 11:37 a.m., Bhushan Shah wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125802/
> -----------------------------------------------------------
> 
> (Updated Oct. 26, 2015, 11:37 a.m.)
> 
> 
> Review request for Plasma, David Edmundson and Martin Gräßlin.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> -------
> 
> Advantage of this split is some parts such as showLockWindow and hideLockWindow needs platform specific code. So instead of doing complex if/else branching we can have seperate X11Locker and WaylandLocker.
> 
> 
> Diffs
> -----
> 
>   ksmserver/screenlocker/CMakeLists.txt 24a89f6 
>   ksmserver/screenlocker/abstractlocker.h PRE-CREATION 
>   ksmserver/screenlocker/abstractlocker.cpp PRE-CREATION 
>   ksmserver/screenlocker/autotests/CMakeLists.txt ab901ca 
>   ksmserver/screenlocker/autotests/lockwindowtest.cpp 444c44f 
>   ksmserver/screenlocker/autotests/x11lockertest.cpp PRE-CREATION 
>   ksmserver/screenlocker/ksldapp.h b281ab6 
>   ksmserver/screenlocker/ksldapp.cpp 68ebfc2 
>   ksmserver/screenlocker/lockwindow.h c4013be 
>   ksmserver/screenlocker/lockwindow.cpp e46733e 
> 
> Diff: https://git.reviewboard.kde.org/r/125802/diff/
> 
> 
> Testing
> -------
> 
> builds, installs, autotests and kscreenlocker_test passes, normal lockscreen also works.
> 
> 
> Thanks,
> 
> Bhushan Shah
> 
>

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