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
