On Saturday 10 August 2013 17:10:00 Jan Kundrát wrote: > Hi, a couple of comments: > > - The tests should be built by default. That's the whole point > of them -- whenever someone compiles Trojita (and > *especially* when someone makes any change), the tests must > be run. >
OK.
> - The build should abort when someone attempts to build with
> KDE and Qt5 support at the same time; there are no KDE
> libraries with support for Qt5 yet.
>
Ok, I will rewrite cmake to use cmake_dependent_option.
> - I'm not sure about the WITH_RAGEL option -- I'd prefer to
> have it tri-state, "force on", "force off" and "autodetect"
> with defaulting to "autodetect". The same applies to
> WITH_ZLIB.
>
What about to have two options OFF and AUTO? And write status if
feature will be compiled or not (depending on find_package)?
> - WITH_KDE, WITH_AKONADI and WITH_KONTACT should not default
> to "on"; either use an automagic detection or leave them off
> by default. I do not plan to deal with "but now it requires
> KDE" support e-mails.
>
Ok.
>
> - It seems that the cmake options do not work at all; when I
> run `cmake -DWITH_KDE=1` in a clean build dir, no cmake
> messages about QtKeychain are output and the build later dies
> with an error when including it. I don't have that library on
> my system.
>
I will add options for enabling/disabing each plugin.
> - I see that FindKDE4Internal.cmake is rather creative in its
> clobbering of CMAKE_CXX_FLAGS (which you partially undo when
> enabling exceptions). I have mixed opinions here -- the
> increased warnings are OK, both -fno-check-new and
> -fno-common are either harmless or good, and only the
> exceptions are a problem. Still, I'm considering building the
> KDE-specific plugins within a subdirectory so that these
> KDE-specific flags do not affect the rest of Trojita.
>
I think now compilation working fine and ${KDE4_ENABLE_EXCEPTIONS}
is there for enabling exceptions in kde4 apps.
> - There should be a way of building *just the plugins*, or
> even better, just a single plugin at a time. This will be
> important for packaging where each plugin should go into a
> separate package.
>
> Cheers,
> Jan
--
Pali Rohár
pali.ro...@gmail.com
signature.asc
Description: This is a digitally signed message part.
