[cmake-developers] Compact dependency graph

Nils Gladitz nilsgladitz at gmail.com
Tue Jun 16 03:03:53 EDT 2015


On 06/15/2015 09:46 PM, Richard Ulrich wrote:
> So here is the patch with tests and passing the git hooks.

Thank you for the update and sorry again for the inconvenience.

- Your commit's subject line looks a bit too long; I'd move the issue 
number into the bulk of the message (It should nicely fit into the 
console in "git log").

- In RegexReplacerCallback you've got lowercase members that start with 
underscore. I'd change that to follow the same convention you used in 
e.g. RegexReplacer (camel case starting upper case).

- I think the tests visible to ctest are a bit too granular. I would 
subsume related tests into one ctest visible test (e.g. as is done for 
CommandLineTar).

- The RunCMake.graphviz_no_filter and RunCMake.graphviz_filter_even 
tests currently seem to fail for me.

Comparing expected.dot with the actual output of e.g. 
RunCMake.graphviz_no_filter shows basically the same content but the 
node numbering is entirely different.

I'd try to see if either node numbering could be made consistent (I am 
not sure why it isn't) or use regular expression matching on the 
expected results instead (as is e.g. done by RunCMake for stderr/stdout).

Something like ...
  file(READ ... actual)
  file(READ ... expected)

  if(NOT actual MATCHES "${expected}")
	message(FATAL_ERROR "... ${actual} ... ${expected}")
  endif()

Nils


More information about the cmake-developers mailing list