KDE/kdelibs/cmake/modules

[Alexander Neundorf]

On Thursday 03 September 2009, Dario Freddi wrote:
> In data giovedì 03 settembre 2009 19:35:30, Alexander Neundorf ha scritto:
...
> > * 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

Hmm, yeah, trying to keep an eye on kdelibs is already enough work...
Also I'm not sure what compatibility guarantees we give for 
kdebase/workspace/.

> > 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

I guess this means ok ?

> > * 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)

It's a macro coming with cmake and used by many modules.  See the cmake 
manpage.

> > * 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?

The variables is set, but neither documented nor used.
Maybe just remove it ?
If it stays in that file, it should be documented and it really should get the 
POLKITQT_ prefix, so it is clear where it comes from.

Where is POLKITQT_PREFIX set ?

> > 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) 

Can you please check where this comes from ?
Should be FindDBus.cmake...

> 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. 

FindDBus.cmake looks like a good candidate ;-)
(this depends on where it comes from, not who uses it, the same as all classes 
coming from Qt start with "Q", even if they are used in KDE "K" 
programs ;-) )

Alex