lordgamez commented on code in PR #1734: URL: https://github.com/apache/nifi-minifi-cpp/pull/1734#discussion_r1509271721
########## cmake/MiNiFiOptions.cmake: ########## @@ -82,6 +80,8 @@ list(APPEND STRICT_GSL_CHECKS_Values AUDIT ON DEBUG_ONLY OFF) set_property(CACHE STRICT_GSL_CHECKS PROPERTY STRINGS ${STRICT_GSL_CHECKS_Values}) if (WIN32) + add_minifi_option(INSTALLER_MERGE_MODULES "Creates installer with merge modules" OFF) + add_minifi_option(INSTALLER_WITH_VC_REDISTRIBUTABLES "Creates installer with Visual C++ redistributables included" OFF) Review Comment: I agree the naming should be changed, to be more clear, and clarify which option bundles what artifacts exactly. It also needs to be specified if a bundle falls under a different license. On the other hand I would keep all three bundle options separate. The rationale behind this version is it's easier and more direct to specify a single option for each installer bundle option instead of combining build options like `INCLUDE_MS_BLOBS` + `USE_UCRT_DLLS`. So I would keep them separately like `INCLUDE_UCRT_DLLS`, `INCLUDE_VC_REDIST_DLLS`, and `INCLUDE_VC_REDIST_MERGE_MODULES`. This way you can directly specify which libraries you want to include in the installer, and by not specifying anything, no Microsoft blobs will be included. What are your thoughts on this? -- 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. To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org