KHealthCertificate in kdereview
Friedrich W. H. Kossebau
kossebau at kde.org
Sun Jul 11 15:07:37 BST 2021
Quick feedback after looking at the cmake code:
* do not explicitly list ECM_KDE_MODULE_DIR, it is part of ECM_MODULE_PATH
* do not use KDEFrameworkCompilerSettings for non-KF projects, only
KDECompilerSettings
* no need to set CMAKE_AUTOMOC & CMAKE_AUTORCC, done by KDECompilerSettings
for required ECM
* set(CMAKE_CXX_STANDARD 17) before including KDECompilerSettings
* call feature_summary as last thing, for consistency and some logic in
execution
* avoid REQUIRED in find_package() calls, only use in set_package_properties,
so cmake will not cancel in middle of run, but get to listing found/notfound
summary (Qt, ECM, KF are harder to do here due to macros expected later, and
usually present, so fine there)
* include the 3 KDE cmake setting modules as first and in this order:
include(KDEInstallDirs)
include(KDECMakeSettings)
include(KDECompilerSettings NO_POLICY_SCOPE)
* I recommend using set_target_properties() right next to add_${target},
easier to find on need and product definition in one place makes more sense
Otherwise looks clean and modern to me :)
Cheers
Friedrich
More information about the kde-core-devel
mailing list