szaszm commented on code in PR #1749: URL: https://github.com/apache/nifi-minifi-cpp/pull/1749#discussion_r1569036566
########## README.md: ########## @@ -185,7 +184,7 @@ On all distributions please use -DUSE_SHARED_LIBS=OFF to statically link zlib, l #### Libraries * libuuid * librocksdb (built and statically linked) -* libcurl-openssl (If not available or desired, NSS will be used) +* libcurl-openssl Review Comment: I didn't notice line 186, but the same applies there. We shouldn't remove the listed dependencies, even if a user doesn't need to install them. Those that don't need to be installed used to be marked with `(built and statically linked)`, meaning no system version is necessary, but users know that code from those libraries is used. If you want to remove them from this list, then we need to add them somewhere else. If you want to do the addition of a second list in MINIFICPP-2331, then you should also do the removal from the existing list there. I understand that the list is not perfect at the moment, but I believe this half-change in 52c35a6334740c874f3515e3e95448eaeca8f143 makes it worse by removing even more information. I see a few valid options for us in the context of this PR: 1. Mark openssl and curl as `(built and statically linked)`, and not touch the rest of the entries 2. Same as 1, plus do the same for other entries that have been recently changed. If something is now built and statically linked by our build system, mark it as such. If something is no longer used, remove it. 3. Do all changes described in [MINIFICPP-2331](https://issues.apache.org/jira/browse/MINIFICPP-2331) that are relevant for these two lists. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
