Will Dicharry wrote: > Sorry for the month of delay, but I've addressed Mike Jackson's concerns > below and I think I'm close to having the HDF5 find module ready for > submission.
Excellent. I have a few comments from quickly glancing at them, but I don't have time for thorough testing. Overall it looks good. > There are two modules attached to this message: The FindHDF5.cmake > module and an AdjustLibraryVariables.cmake module, which is essentially > a copy of what the FindQt4 module does. It seems useful enough to > incorporate into the CMake Modules, and I can maintain it if you need a > maintainer. I'd like to choose a better name for AdjustLibraryVariables. Perhaps "SelectLibraryConfigurations"? Does it have all the functionality needed to update FindQt4 to use it too (you don't need to do this but it should be easy for the FindQt4 maintainer to do it)? The find_path and find_library calls need some tweaking. Please read the documentation of these commands to distinguish the cases of PATHS and HINTS keywords. The PATHS should only be last-resort guesses. The HINTS should be locations computed from the system, such as those reported by the hdf5 compiler wrapper tools. Also, paths like /usr/local/include /usr/include are searched automatically and need not be listed. > How I addressed Mike Jackson's concerns is addressed in the module > documentation at the top of the file, please let me know if anyone has > any other concerns. Try placing these modules in the CMake/Modules source tree and running cmake --help-module FindHDF5 cmake --help-module AdjustLibraryVariables to make sure the documentation formats correctly. Also, any macro in the public interface of the module should be documented using a format similar to the CMake command documentation. Thanks, -Brad _______________________________________________ Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://www.cmake.org/mailman/listinfo/cmake