----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126101/#review88635 -----------------------------------------------------------
Ship it! Looks good! Some minor comments left, but no need to bother with another round of review for those, just fix them before committing. Thanks a lot for the patch! backends/fake/fake.cpp (line 96) <https://git.reviewboard.kde.org/r/126101/#comment60754> qCDebug or remove backends/fake/fake.cpp (line 135) <https://git.reviewboard.kde.org/r/126101/#comment60755> qCDebug or remove, fix indent src/backendmanager.cpp (line 185) <https://git.reviewboard.kde.org/r/126101/#comment60759> "Use the static version of loadBackend instead." -> "Use loadBackendPlugin() instead." src/backendmanager.cpp (line 361) <https://git.reviewboard.kde.org/r/126101/#comment60760> Instead of deleting the AbstractBackend manually here, you could let QPluginLoader do it by `mLoader->unload()`, then delete mLoader normally (not via deleteLater()). As a bonus it will unload the plugin binary from the process (just deleting QPluginLoader does not). src/setconfigoperation.cpp (line 58) <https://git.reviewboard.kde.org/r/126101/#comment60758> Remove - Daniel Vrátil On Nov. 20, 2015, 1:16 a.m., Sebastian Kügler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/126101/ > ----------------------------------------------------------- > > (Updated Nov. 20, 2015, 1:16 a.m.) > > > Review request for Plasma, Solid, Àlex Fiestas, Aleix Pol Gonzalez, Daniel > Vrátil, and Martin Gräßlin. > > > Repository: libkscreen > > > Description > ------- > > This patchset adds in-process operation to libkscreen > > purpose: > - allow easier debugging > - for some backends (qscreen, upcoming kwayland) the out of process operation > is not necessary since these backends are well-shielded > > This implementation is backwards compatible and opt-in for running setups. > > If the user exports KSCREEN_BACKEND_INPROCESS=1 before running, all > operations will be done in process. Otherwise, the out-of-process mode is > used. > > Changes revolve around the ConfigOperations, the BackendManager (which > "factories" the backends, either in- or out-of-process), and the plugin > loading logic > > Autotests should cover all the cases (and actually a few currently > unsupported ones, such as using different backends in the same process). > > Details on performance, etc.: > http://vizzzion.org/blog/2015/11/wayland-and-libkscreen-benchmarks/ > > > Diffs > ----- > > CMakeLists.txt 86a0965 > autotests/CMakeLists.txt 69af7f0 > autotests/testconfigmonitor.cpp a051226 > autotests/testinprocess.cpp PRE-CREATION > autotests/testqscreenbackend.cpp da4dbae > autotests/testscreenconfig.cpp ecbcedf > autotests/testxrandr.cpp b9b838a > backends/fake/fake.cpp 60264dd > src/backendlauncher/backendloader.cpp 52051df > src/backendmanager.cpp ca9c746 > src/backendmanager_p.h c6418e2 > src/config.cpp 75d947d > src/configmonitor.h b6f1189 > src/configmonitor.cpp a14bc70 > src/configoperation.h 2405d79 > src/configoperation.cpp 87fe141 > src/configoperation_p.h afdc462 > src/getconfigoperation.h c85bfaa > src/getconfigoperation.cpp 22f92b4 > src/output.cpp bd381fa > src/setconfigoperation.h 020a990 > src/setconfigoperation.cpp 6ea944f > > Diff: https://git.reviewboard.kde.org/r/126101/diff/ > > > Testing > ------- > > Added a ton of autotests, made sure all existing ones pass. > > tried "KSCREEN_BACKEND_INPROCESS=1 kcmshell5 kscreen", all working as > expected. > > > Thanks, > > Sebastian Kügler > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
