KDE/kdelibs/cmake/modules

[Dario Freddi]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part.
Url : http://mail.kde.org/pipermail/kde-buildsystem/attachments/20090903/b4ddfba5/attachment-0001.sig