Review Request 123722: Add PROPERTIES argument to ecm_add_test and ecm_add_tests.

Alex Merry alex.merry at kde.org
Sat May 16 09:28:11 UTC 2015


On Thursday 14 May 2015 17:01:22 Stephen Kelly wrote:
> Alex Merry wrote:
> > Add PROPERTIES argument to ecm_add_test and ecm_add_tests.
> 
> I know you have merged this already, but I consider it a bad design. If the
> user wants to set properties, then the user can use set_test_properties().
> 
> Using more commands is good and making command parameters all-powerful is
> bad. It's exactly what we moved away from by porting away from
> kde4_add_{executable,library}. You're taking us back down that road with
> this change. I suggest reverting it.
> 
> If the user wants to set a property, then they use the 'native' cmake
> command to do so. That is inline with the principle we used when starting
> kf5 and started upstreaming things to cmake. It leads to more understandable
> code, whereas macros/functions with 'combined everything' parameters are
> less understandable.

Good point. We still need to solve the underlying problem that, as you pass 
source files (rather than test or target names) to the command, it would be 
pretty clunky to gather the arguments you need for  set_test_properties().

A better (and more generic) solution would probably be to be able to pass in a 
variable name to receive the generated test names.

Alex


More information about the Kde-buildsystem mailing list