Review: Needs Fixing

I don't understand what ZORBA_WITH_FOP means at all. I'm not really a fan of 
having flags to enable functionality anyway, at least not in non-core module 
packages - if the user took the time to download the package, presumably they 
want the contents, so why force them to add another CMake flag to enable it?

But with this change, ZORBA_WITH_FOP is completely meaningless, because it's 
now SET(ON FORCE). It will always be ON. So the IF(ZORBA_WITH_FOP) is pointless.

I would suggest eliminating the flag entirely. If we really think we need a way 
to build the data-formatting module without FOP, have a ZORBA_SUPPRESS_FOP flag 
instead that skips the whole file.

Cezar: Rodolfo had a comment about the nested IF thing - apparently IF(EXISTS a 
AND EXISTS b...) doesn't work, possibly a CMake bug. I haven't verified that 
bug myself, but it wouldn't surprise me; I've certainly seen other situations 
where IF(... AND ...) doesn't do what I'd expect in CMake.
-- 
https://code.launchpad.net/~zorba-coders/zorba/bug955040/+merge/98891
Your team Zorba Coders is subscribed to branch lp:zorba/data-formatting-module.

-- 
Mailing list: https://launchpad.net/~zorba-coders
Post to     : zorba-coders@lists.launchpad.net
Unsubscribe : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp

Reply via email to