-----------------------------------------------------------
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
> 
>

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to