[cmake-developers] Introduction and volunteering for the Matlab package

[Brad King]

On 01/22/2015 11:50 AM, Raffi Enficiaud wrote:
> I can also do a pull request if you prefer, 

As described in CONTRIBUTING.rst a patch here is preferred.

I've fetched your branch from https://github.com/raffienficiaud/CMake
Here are some comments.

Please wrap text in the documentation blocks of the FindMatlab
module to fit in 79 columns.

> #    find_package(Matlab
> #                 [VERSION]
> #                 [REQUIRED]
> #                 [COMPONENTS component1 [component2]])

Individual find modules don't need to summarize the find_package
signature.  Documentation of components and versions can just
refer to the :command:`find_package` command and name the options.

> # .. note:
> #
> # The version above is the Matlab version...

The note text needs to be indented to be part of the note.  The same
goes for all the variable and command documentation inside explicit
markup directives like ".. variable::" and ".. command::".

> # User defined variables
> # ----------------------

This section should be called something like "Module Input Variables"
and have intro wording like "Users or projects may set the following
variables to configure this module behavior.".

> # Variables defined by the module
> # -------------------------------

This section should distinguish between cache entries and output
variables.  See FindJsonCpp for an example.

> # Provided macros
> # ---------------

Generally we try to use functions with "set(... PARENT_SCOPE)".

Also on the endmacro() and endfunction() calls please drop the
command name.

> # WARNING: this thing pollutes the CMAKE_FIND_LIBRARY_PREFIXES global variable. 
> # Should it be restored afterwards? Is there a more appropriate way to do that?
> set(CMAKE_FIND_LIBRARY_PREFIXES ${CMAKE_FIND_LIBRARY_PREFIXES} ${_matlab_lib_prefix_for_search})

This should be handled with a save/restore.

> # The function arguments are:

Please use definition lists instead of bullet lists for function
argument documentation.

The copyright year should be 2014-2015 since we've spilled into
that range.  There is no need for copyright notices in the
Tests/RunCMake test cmake code because there no creative input
in that boilerplate.

Please remove all trailing whitespace and make sure the files are
committed with LF newlines and not CRLF newlines (including tests).
Also make sure all files end in a newline (but not trailing blank
lines).

Thanks,
-Brad