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