Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17554 )
Change subject: [rest] add oat++ framework to kudu ...................................................................... Patch Set 17: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/17554/16/cmake_modules/FindOatpp.cmake File cmake_modules/FindOatpp.cmake: http://gerrit.cloudera.org:8080/#/c/17554/16/cmake_modules/FindOatpp.cmake@24 PS16, Line 24: oatpp-1.2.5 > It is possible but I have to change the CmakeLists.txt for the Oat++ and th I guess a better approach would be adding a patch into $KUDU_HOME/thirdparty/patches to update oatpp-<version>/cmake/module-config.cmake.in and oatpp-<version>/cmake/module-install.cmake once the oatpp is unpacked. The patch would remove the version-specific suffixes for oatpp. The idea is to keep all distro tarballs for Kudu's thirdparty as close to the originals as possible, otherwise the sacred knowledge of making the distro tarballs should be recorded elsewhere. Overall, it's surprising to me to see oatpp using version-specific paths to install its header and library files. Maybe, that's designed to be used in oatpp development environments where multiple versions of the library installed simultaneously? If it's more convenient to have those version-related updates for oatpp in a separate follow-up changelist, that's totally fine with me to keep this changelist as-is in this regard and address the version-related issue later on. -- To view, visit http://gerrit.cloudera.org:8080/17554 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie1b5376297b0170a624655acf836cdedc090f6e7 Gerrit-Change-Number: 17554 Gerrit-PatchSet: 17 Gerrit-Owner: Khazar Mammadli <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Khazar Mammadli <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 06 Jul 2021 16:57:54 +0000 Gerrit-HasComments: Yes
