DevHeads.net

Review Request: plasma-thunderbolt

Hi all,

plasma-thunderbolt is a new repo containing, you guessed it, Thunderbolt KCM
for Plasma. I initially submitted the code as a patch against plasma-desktop
[0], where it got reviewed, but it was ultimately decided to better put it
into a separate repository, since it's not just a KCM but also a library and a
KDED module. I have backported all the changes from the Phabricator review
back to the repository, so the code in the repo is identical to the one in the
Phab review (minus buildsystem changes and a small build fix for clang).

However, since this is still a new code, it must formally pass through
kdereview before I can submit it into Plasma as a new module.

Thus I'd kindly ask you to take one more look at the codebase [1] and let me
know if there are any more issues to fix, or if we can proceed to include this
in the next Plasma release.

Thanks,
- Dan

[0] <a href="https://phabricator.kde.org/D19011" title="https://phabricator.kde.org/D19011">https://phabricator.kde.org/D19011</a>
[1] <a href="https://cgit.kde.org/plasma-thunderbolt.git" title="https://cgit.kde.org/plasma-thunderbolt.git">https://cgit.kde.org/plasma-thunderbolt.git</a>

Comments

Re: Review Request: plasma-thunderbolt

By Friedrich W. H.... at 05/15/2019 - 09:55

Am Mittwoch, 15. Mai 2019, 15:27:07 CEST schrieb Daniel Vrátil:
Pushed some small fixes to toplevel CMakeLists.txt

Other things seen on quick look at code (also not tested runtime):
* kded misses a Messages.sh file.
* no COPYRIGHT license files in the repo
* kde_enable_exceptions() duplicated a few times, perhaps only do in subdirs
where needed or use of kde_target_enable_exceptions() if fitting
* libkbolt being a private library could be reflected in the libname, also get
install(TARGETS kbolt ${KDE_INSTALL_TARGETS_DEFAULT_ARGS} LIBRARY
NAMELINK_SKIP)

Cheers
Friedrich