Including LayerShellQt in Plasma in time for 5.22
Aleix Pol
aleixpol at kde.org
Tue Apr 6 12:59:40 BST 2021
On Tue, Apr 6, 2021 at 11:08 AM Harald Sitter <sitter at kde.org> wrote:
> - I'm like a broken record at this point: some of the cmake files and
> the json file lack licensing details. please run `reuse lint` to check
> the list. it's incredibly close to being fully licensed
> - you include KDEClangFormat but don't actually seem to enable it
> - along a similar note: the code style is inconsistent at times, you
> might want to also enable the githooks when built with a new enough
> ECM
> - CheckIncludeFiles is also included but not used
> - you sometimes use the Qt5::Foo targets rather than the Qt::Foo
> targets, I believe the latter is preferred for easier porting to qt6
> - the library code calls qputenv and ignores its return value, might
> make sense to qcwarning when the env variable failed to set
>
> other than that it looks lovely 👍
>
> HS
>
I think all the issues brought up here have been addressed.
Thanks everyone!
Aleix
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20210406/0572a777/attachment.htm>
More information about the kde-core-devel
mailing list