... sending it again, since I assume that Dario is not subscribed to both lists
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 _______________________________________________ Kde-buildsystem mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-buildsystem
