rleigh-codelibre commented on a change in pull request #2: URL: https://github.com/apache/xerces-c/pull/2#discussion_r443149185
########## File path: CMakeLists.txt ########## @@ -177,8 +177,14 @@ install( # Process subdirectories add_subdirectory(doc) add_subdirectory(src) -add_subdirectory(tests) -add_subdirectory(samples) +if (NOT (DEFINED XERCES_BUILD_FUZZERS)) Review comment: I would prefer that the fuzzer keep its conditionals self-contained if possible. That is, rather than disabling other unrelated features, it should only enable or disable the fuzzing functionality. If you want to disable building of the tests and/or samples, I think they should have separate options to allow them to be enabled or disabled independently. You could then do `-Dtests=OFF -Dsamples-OFF -Dfuzzer=ON` to achieve the same effect. Also, the extra compile-time stuff using ExternalProject looks separate from building the fuzzer itself. Maybe always build the fuzzer but have that additional stuff gated on the `fuzzer` option. Or gate all of it. But I would put that conditional stuff *inside* the fuzzer CMakeLists.txt rather than having any conditionals around include statements. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: c-dev-unsubscr...@xerces.apache.org For additional commands, e-mail: c-dev-h...@xerces.apache.org