Review Request 127387: Refactor the backend loading code

Martin Gräßlin mgraesslin at kde.org
Wed Mar 16 13:59:21 UTC 2016


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




autotests/testbackendloader.cpp (line 46)
<https://git.reviewboard.kde.org/r/127387/#comment63829>

    variable is no longer used. It's written to but never read.



src/backendmanager.cpp (line 77)
<https://git.reviewboard.kde.org/r/127387/#comment63830>

    Now we can even make the list a const \o/



src/backendmanager.cpp (line 161)
<https://git.reviewboard.kde.org/r/127387/#comment63831>

    QStringLiteral


- Martin Gräßlin


On March 16, 2016, 2:54 p.m., Sebastian Kügler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127387/
> -----------------------------------------------------------
> 
> (Updated March 16, 2016, 2:54 p.m.)
> 
> 
> Review request for Plasma, Solid, Daniel Vrátil, and Martin Gräßlin.
> 
> 
> Repository: libkscreen
> 
> 
> Description
> -------
> 
> Refactor the backend loading code
>     
>     Untangle the large plugin loading logic in the BackendManager static
>     into  separate bits. This makes the code clearer and easier to auto-test.
>     
>     - listBackends() compiles a list of backends from the plugin paths
>     - preferredBackend() picks the backend, in this priority:
>         - if KSCREEN_BACKEND is set in the environment, this is the only
>           used method to find the backend plugin
>         - if platform is X11, the XRandR backend is picked
>             - if platform is wayland, KWayland backend is picked
>             - if neither is the case, QScreen backend is picked
>     
>     It does introduce a slight behavioral change: The mechanism was based on
>     falling through, so it would consider another backend if the logically
>     picked on fails to load. This is undesired behavior, however, since the
>     backendloader may be able to load the plugin, but that doesn't mean that
>     the plugin actually work.
>     
>     Parsing of the KSCREEN_BACKEND variable is kept case-insensitive.
> 
> 
>     autotests for new backend loading logic
>     
>     - makes sure we find plugins installed
>     - pick plugins from env var
>     
>     We can't sensible test all the runtime cases yet, but this at least
>     covers the code paths around those few lines that do runtime detection
>     of x11 and wayland.
> 
> 
> Diffs
> -----
> 
>   autotests/CMakeLists.txt 26c7952 
>   autotests/testbackendloader.cpp PRE-CREATION 
>   src/backendmanager.cpp 382ae71 
>   src/backendmanager_p.h 150883b 
> 
> Diff: https://git.reviewboard.kde.org/r/127387/diff/
> 
> 
> Testing
> -------
> 
> manual runtime tests and autotests pass
> 
> 
> Thanks,
> 
> Sebastian Kügler
> 
>

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


More information about the Plasma-devel mailing list