On 10/26/06, Brad King <[EMAIL PROTECTED]> wrote:
Tristan Carel wrote:That looks pretty nice. Here are a few comments: 1.) Please change EXEC_PROGRAM to EXECUTE_PROCESS.
Done.
You can use the new OUTPUT_STRIP_TRAILING_WHITESPACE option in CVS CMake's EXECUTE_PROCESS to avoid extra newlines. Since the module will not be distributed in a cmake without that feature it should work.
I can't access the repository right now, I can't make tests to view the difference but I trust you.
2.) There is a MESSAGE(ERROR) call. This is invalid. It should be MESSAGE(SEND_ERROR).
You're right ... one of my bad habits:/
3.) The proper way to detect if there was a problem with executing a process is to use the RESULT_VARIABLE option for EXECUTE_PROCESS. If it is set to anything other than "0" then there was an error and the variable contains a message describing it or the return value. You can then still use the ERROR_VARIABLE to display other output.
I know, I found a case where `svn info' didn't return 1 whereas the path to the working copy was wrong. But I'm not able to reproduce this case => I changed the ugly test
4.) When using the STRING(REGEX) command the input argument should
always be quoted:
STRING(REGEX REPLACE ...
Subversion_VERSION_SVN ${Subversion_VERSION_SVN})
should be
STRING(REGEX REPLACE ...
Subversion_VERSION_SVN "${Subversion_VERSION_SVN}")
Otherwise if the variable is empty STRING will complain about having too
few arguments.
Do you believe me if I say I'm a shell wizard? :)
5.) There should be no TABs in the source file.
Whooo you use a smart script to check contributors ugliness :)
6.) Why is Subversion_SVN_EXECUTABLE marked as advanced only when the macro is called? Is that a cut-and-paste error?
Yeah, I guess it's an acceptable reason. now the RC2 Thank you -- Tristan Carel Music with dinner is an insult both to the cook and the violinist.
FindSubversion-rc2.cmake
Description: Binary data
_______________________________________________ CMake mailing list [email protected] http://www.cmake.org/mailman/listinfo/cmake
