[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-panel-devel
Subject: Re: Review Request 127387: Refactor the backend loading code
From: Sebastian_Kügler <sebas () kde ! org>
Date: 2017-02-04 11:18:50
Message-ID: 20170204111850.23300.9707 () mimi ! kde ! org
[Download RAW message or body]
--===============3115587091108832287==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127387/
-----------------------------------------------------------
(Updated Feb. 4, 2017, 11:18 a.m.)
Status
------
This change has been marked as submitted.
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
--===============3115587091108832287==
MIME-Version: 1.0
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 8bit
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 \
solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;"> \
<tr> <td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/127387/">https://git.reviewboard.kde.org/r/127387/</a>
</td>
</tr>
</table>
<br />
<table bgcolor="#e0e0e0" width="100%" cellpadding="12" style="border: 1px gray solid; \
border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;"> <tr>
<td>
<h1 style="margin: 0; padding: 0; font-size: 10pt;">This change has been marked as \
submitted.</h1> </td>
</tr>
</table>
<br />
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: \
1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; \
-webkit-border-radius: 6px;"> <tr>
<td>
<div>Review request for Plasma, Solid, Daniel Vrátil, and Martin Gräßlin.</div>
<div>By Sebastian Kügler.</div>
<p style="color: grey;"><i>Updated Feb. 4, 2017, 11:18 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
libkscreen
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">Refactor the backend loading code</p> <p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;"><div class="codehilite" style="background: #f8f8f8"><pre \
style="line-height: 125%"><span></span>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.
</pre></div>
</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">manual runtime tests and autotests pass</p></pre> \
</td> </tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>autotests/CMakeLists.txt <span style="color: grey">(26c7952)</span></li>
<li>autotests/testbackendloader.cpp <span style="color: \
grey">(PRE-CREATION)</span></li>
<li>src/backendmanager.cpp <span style="color: grey">(382ae71)</span></li>
<li>src/backendmanager_p.h <span style="color: grey">(150883b)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/127387/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
--===============3115587091108832287==--
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic