Review Request 127387: Refactor the backend loading code

[Sebastian Kügler]

> On March 15, 2016, 11:42 p.m., David Edmundson wrote:
> > src/backendmanager.cpp, line 216
> > <https://git.reviewboard.kde.org/r/127387/diff/1/?file=453202#file453202line216>
> >
> >     is the name argument still used?
> >     
> >     It looks like that used to be the name of the backend to load, which you now determine yourself inside the function, rendering it useless.

Yes, I want to kill "name", but it's used from a couple of places in the API (but doesn't get used in the end, it's not needed since we share all the logic now with in- and out-of-process loading. I'll remove the remainders of name in a separate patch.


- Sebastian


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


On March 15, 2016, 5:57 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 15, 2016, 5:57 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/b0e6a237/attachment.html>