Review Request 125802: Split generic parts from X11Locker into AbstractLocker

Bhushan Shah bhush94 at gmail.com
Mon Oct 26 12:13:30 UTC 2015



> On Oct. 26, 2015, 5:32 p.m., David Edmundson wrote:
> > ksmserver/screenlocker/abstractlocker.h, line 68
> > <https://git.reviewboard.kde.org/r/125802/diff/3/?file=412805#file412805line68>
> >
> >     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

I will do it later in seperate review request.. currently I've 2-3 other patches depending upon this one and rebasing them would be mess I fear..


- Bhushan


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


On Oct. 26, 2015, 5:07 p.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, 5:07 p.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/debb1f0d/attachment.html>


More information about the Plasma-devel mailing list