In data giovedì 03 settembre 2009 19:35:30, Alexander Neundorf ha scritto: : > ... sending it again, since I assume that Dario is not subscribed to both > lists
You assume right :) > > 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 Nice to know, I'll copy on that. > > 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-con > fig > > * 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-con > fig Will fix both > > * I missed the point where the new files were posted for review on > kde-buildsystem (maybe due to my vacation ?) It was part of KAuth during review. Additionally, that very file (polkitqt.cmake) is in kdebase since 4.3 > > > 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) Roger that > > * please use the find_package_handle_standard_args() macro, if there are no > real reasons against using it Ok (I don't even know the difference, but will search and will do) > > * what's up with POLICY_FILES_INSTALL_DIR and This is the directory where policy fileswill be installed. Given that is related to polkit and not to polkit-qt, I thought it was a better idea to keep the naming this way. Suggestions? > dbus_add_activation_system_service() ? This one is actually a modification of dbus_add_activation_service (that is somewhere else I don't remember now) that adds an activation service on the system bus, not the session one. This was done to keep naming consistent, but could probably be moved somewhere else. Although, since polkit-qt makes use of it and it's the only one, I don't know if it's worth it. Probably it should more belong to KAuth now, since it uses DBus also on other platforms to activate privileged helpers. See also my last mail on k-c-d on this regard (moving kauth macros into kde4macros) > They are not documented Yes, I was told that cmake needs docs as well and will do this as soon as possible > and they > don't follow the naming conventions, i.e. they don't start with > "POLKITQT_" As explained before Thanks for pointing out the issues :) > > Alex > -- ------------------- Dario Freddi KDE Developer GPG Key Signature: 511A9A3B
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ Kde-buildsystem mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-buildsystem
