[cmake-developers] A few topic reviews for Stephen
Brad King
brad.king at kitware.com
Tue May 21 15:48:22 EDT 2013
Stephen,
We reviewed most of your latest topics in 'next' today, merged
many to 'master', and committed trivial fixups to a couple.
However, there are problems with some topics:
* genex-generate-file: Appears to leak memory. It calls
new cmGeneratorExpressionEvaluationFile but has no delete.
* join-genex: I think this is the cause of a VS 6 test failure
of the CompileDefinitions test:
http://open.cdash.org/testDetails.php?test=190944806&build=2911585
WARNING: The VS6 IDE does not support preprocessor definition values with spaces and '"', '$', or ';'.
CMake is dropping a preprocessor definition: CMAKE_IS_REALLY="Very Fun"
* language-generator-expression: based on join-genex, see above
* generate_export_header-fixes: See general request/advice below.
I updated commit messages in the short topics but this one is
longer. Please revise the commit messages of the entire topic.
A general request to make reviews easier: please follow more
strictly the practice of prefixing commit messages with an
"area: " header at the beginning of the first line. Imagine
reading the commit message without separately knowing the
list of files it changes. The "area: " and description should
together make it possible to infer where the change should be.
When writing a commit message I like to pretend I'm writing an
email to someone that instructs them to write the patch. The
the commit message summary (first) line is the email subject.
The rest is the body. The imperative mood instructs the reader
to make the change. When reading a patch it should be plausible
that it was constructed by a competent developer just from the
commit message. This makes review easier because we can check
if the change does what the message says rather than trying to
infer the intention and correctness of the change just from the
patch.
Thanks,
-Brad
More information about the cmake-developers
mailing list