On 03/05/2014 05:46 PM, Raffi Enficiaud wrote: > So, I began writing the doc of the module and changed the license > according to the instructions. I am attaching the module to the email.
Thanks! The documentation so far is a good start but it looks like you're still working on it. You can copy the module into CMake on top of the current FindMatlab.cmake locally, install sphinx-build, and then enable SPHINX_HTML in your local CMake build tree to build the documentation. It will appear in Utilities/Sphinx/html. > - the error messages are more or less message(STATUS... and sometimes > message(FATAL_ERROR, rather than using Xxx_NOT_FOUND_MESSAGE. > So basically if I set this variable to the error message and just > call “return()” from the module scope, it will do the job? The <name>_NOT_FOUND_MESSAGE variable is a feature of the helper module "FindPackageHandleStandardArgs". Use that module's macro at the end of your module to do all the error handling and to set MATLAB_FOUND automatically. Most of the STATUS messages should either be dropped or conditioned on the QUIET option of the invoking find_package command. Some complex find modules like FindBoost have a special variable the user can set to get verbose debug output, e.g. if(MATLAB_FIND_DEBUG) message(STATUS ...) endif() > - it looks like I am making too much use of the CACHE. > However, I do not know if the variables like Xxx_INCLUDE_DIRS > should go to the CACHE or to the PARENT_SCOPE. The singular names that have find_* command calls will be cached by those commands. Plural names like Xxx_INCLUDE_DIRS should just be set as normal variables with set() or list(APPEND), etc. They do not need PARENT_SCOPE unless you are using an internal helper function. CACHE entries should be used only for things that an end user might need to set or change to point at specific things on their system. > - some of the variables should be renamed Yes. Also local variables like "64Build" should be renamed to have a prefix like "_matlab_" and then unset() at the end. > - the version matching should be performed in the module, right ? Yes. I haven't read through the module thoroughly yet but please check that it will be compatible with the exiting FindMatlab.cmake that it is replacing. By "compatible" I mean that the same result variables must be made available to the calling project, even if documented as deprecated. 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://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers