On 12/14/2010 04:45 PM, Alexander Neundorf wrote: > I merged this now into next on stage. > It contains: > - the new policy CMP0017 > - the feature to set the default via CMAKE_POLICY_DEFAULT_CMP<NNNN> > - a basic test > - some documentation > > Please have a look at it.
Good start. Comments below. The implementation in cmMakefile::GetModulesFile should be able to check whether the includer is under CMAKE_ROOT and not bother with the double lookup and policy check unless necessary. + "CMAKE_MODULE_PATH afterwards. This behaviour is controlled by policy " + "CMP0017." I think this should just be "See also policy CMP0017". I don't want to imply that it should be switched for anything but compatibility. The main hunk for CMAKE_POLICY_DEFAULT_CMP<NNNN> should be factored out into a helper method. Also, the error message when this variable is set wrong should mention the name of the variable. In the CMP0017 test: +FIND_PACKAGE(ZLIB) use QUIET since we don't care whether ZLIB is found. The version number documented in the policy is 2.8.3, but it should be 2.8.4. Also, the policy documentation IMO should not use "CMAKE_ROOT" as that may not be meaningful to everyone. Please reword it to use "builtin module directory" or something like that. Thanks, -Brad _______________________________________________ cmake-developers mailing list cmake-developers@cmake.org http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
