On Tue, Apr 6, 2021 at 11:08 AM Harald Sitter <sit...@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
