Review Request: properly pass the NOGUI flag

[Stephen Kelly]

On Tuesday, July 31, 2012 20:34:56 Alexander Neundorf wrote:
> Not sure I like that. This is again a wrapper around one function call:
> 
> set_target_properties(${_target}
>                       PROPERTIES
>                       WIN32_EXECUTABLE FALSE
>                       MACOSX_BUNDLE FALSE
>                      )

Yes. We can either copy/paste the above where needed, or have some kind of 
abstraction. One available abstraction is a wrapper macro/function.

> 
> Can we do this in some other way ?
> Somewhere do something like this:
> set(TARGET_NOGUI_PROPERTY WIN32_EXECUTABLE FALSE
>                           MACOSX_BUNDLE FALSE)
> 
> and then use it:
> set_target_properties(${_target}
>                       PROPERTIES ${TARGET_NOGUI_PROPERTY}
>                      )
> 
> Not sure this is easier.

I'm not sure either. I'm happy with how it is, but I'm also happy to leave the 
choice up to you :)

> 
> We had a nicer API with our kde4_add_executable(...) macro. There we could
> add switches as we wanted.

I prefer the wrapper function to kde4_add_executable (whos name doesn't really 
suggest all of the things it does, other than a very fuzzy 'add an executable 
in the KDE-way'), but API is subjective anyway I guess.

> I don't think we should add some macro for setting a single target property.

In general? I don't see why that's a problem in general. It's like saying 
include_directories() is a bad idea because you can just append to the 
directory property to include directories instead. Actually 
include_directories() is something I find more readable and I don't have to 
remember the syntax and position of the PROPERTIES keyword etc of the property 
setting commands.

That's why I prefer the wrapper, but your solution is also not unlike the 
solution we already have with INSTALL_TARGET_DEFAULT_ARGS. With 
INSTALL_TARGET_DEFAULT_ARGS I always copy-pasted the line from elsewhere, but 
I can remember function names and just write them without fear of typos or 
forgetting to write PROPERTIES :).

> Also, when adding new files to e-c-m, its version number should be increased
> and the higher version should be made required where it is used.

Yes, I'm generally too lazy for that. I know that's one solution to the CMake 
version preference problem, but the lazier solution is to not bump the 
version, not bump the version requirement in the N places it is stated in KF5, 
and expect people hitting errors at cmake time to know to update their ecm.

A simple git pull && make install is all anyone else (and I) have to do to get 
it into the same prefix that KF5 is already looking at.

At least until we get closer to a KF5 release, I'm too lazy to do version 
bumps for every change I add to ECM. 

Thanks,

-- 
Stephen Kelly <stephen.kelly at kdab.com> | Software Engineer
KDAB (Deutschland) GmbH & Co.KG, a KDAB Group Company
www.kdab.com || Germany +49-30-521325470 || Sweden (HQ) +46-563-540090
KDAB - Qt Experts - Platform-Independent Software Solutions
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-buildsystem/attachments/20120802/4c96e0fe/attachment.sig>