> On June 13, 2011, 8:05 a.m., Alexander Neundorf wrote: > > Looks good in general. > > > > The foreach() loop uses new features from cmake 2.8.0, and KDE depends on > > cmake 2.6.4 (later on this year we will require probably cmake 2.8.6, then > > it will be fine), so this must not be used. > > Can you change the foreach()-loop to the basic syntax ? > > Also, the message() which is printed in the body of the loop shouldn't be > > there, a module should only print its results, all the rest should be for > > debugging only. > > > > Alex
Could you elaborate a bit on what is wrong with the foreach loop? According to the documentation for CMake 2.6 it also supports range-based for loops, at least that is what http://www.cmake.org/cmake/help/cmake2.6docs.html#command:foreach tells me. - Arjen ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/101568/#review3859 ----------------------------------------------------------- On June 10, 2011, 7:18 p.m., Arjen Hiemstra wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/101568/ > ----------------------------------------------------------- > > (Updated June 10, 2011, 7:18 p.m.) > > > Review request for kdelibs and Sebastian Kügler. > > > Summary > ------- > > As requested by Sebas, this patch adds a FindQtMobililty.cmake that can be > used to find QtMobility related files. It has support for minimum versions > and searching for individual components. > > > Diffs > ----- > > cmake/modules-tests/QtMobility/CMakeLists.txt PRE-CREATION > cmake/modules/FindQtMobility.cmake PRE-CREATION > > Diff: http://git.reviewboard.kde.org/r/101568/diff > > > Testing > ------- > > There is a simple testcase included. Furthermore I have tested the minimum > version and component related options with a local test file. > > > Thanks, > > Arjen > >
