KDE/kdelibs/cmake/modules
Alexander Neundorf
neundorf at kde.org
Wed Sep 2 19:24:35 CEST 2009
On Wednesday 02 September 2009, Dario Freddi wrote:
> SVN commit 1018860 by dafre:
>
> Fix installation prefix, still need to spit a warning
>
>
> M +4 -1 FindPolkitQt.cmake
CMake files which go into kdelibs/cmake/modules/ must follow the commit policy
we have for them: http://techbase.kde.org/Policies/CMake_Commit_Policy , §7
says "All patches must follow the coding style for CMake files in KDE.",
which is here: http://techbase.kde.org/Policies/CMake_Coding_Style
The PK stuff does not follow these rules.
There are several issues, please fix them:
* it works only if pkg_config works:
http://techbase.kde.org/Policies/CMake_Coding_Style#.28Not.29_Using_pkg-config
* it doesn't use a special prefix for the pkg_config variables, this can break
stuff and lead to ugly effects:
http://techbase.kde.org/Policies/CMake_Coding_Style#.28Not.29_Using_pkg-config
* I missed the point where the new files were posted for review on
kde-buildsystem (maybe due to my vacation ?)
Some things which should be improved (but which are not in the style guide,
but maybe should be):
* please license the cmake files under BSD license, as all other our cmake
files are (reason: cmake is completely BSD licensed, no GPL/LGPL-licensed
files will be accepted into cmake cvs)
* please use the find_package_handle_standard_args() macro, if there are no
real reasons against using it
* what's up with POLICY_FILES_INSTALL_DIR and
dbus_add_activation_system_service() ? They are not documented and they don't
follow the naming conventions, i.e. they don't start with "POLKITQT_"
Alex
More information about the Kde-buildsystem
mailing list