Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17554 )
Change subject: [rest] add oat++ framework to kudu ...................................................................... Patch Set 16: (8 comments) 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 Is there a way to use version-agnostic path here? Using version-specific path might be very inconvenient when the package is updated for a new version in future. I'd expect these path to point to files placed under $KUDU_ROOT/thirdparty/installed/{common,tsan,uninstrumented}, so no version would be in the path? http://gerrit.cloudera.org:8080/#/c/17554/16/cmake_modules/FindOatpp.cmake@29 PS16, Line 29: oatpp-1.2.5 ditto http://gerrit.cloudera.org:8080/#/c/17554/16/cmake_modules/FindOatpp.cmake@31 PS16, Line 31: oatpp-1.2.5/ ditto http://gerrit.cloudera.org:8080/#/c/17554/16/cmake_modules/FindOatppSwagger.cmake File cmake_modules/FindOatppSwagger.cmake: http://gerrit.cloudera.org:8080/#/c/17554/16/cmake_modules/FindOatppSwagger.cmake@24 PS16, Line 24: oatpp-1.2.5 ditto: is it possible to switch to a version-agnostic path here? http://gerrit.cloudera.org:8080/#/c/17554/16/cmake_modules/FindOatppSwagger.cmake@29 PS16, Line 29: oatpp-1.2.5 ditto http://gerrit.cloudera.org:8080/#/c/17554/16/cmake_modules/FindOatppSwagger.cmake@31 PS16, Line 31: oatpp-1.2.5 ditto http://gerrit.cloudera.org:8080/#/c/17554/16/thirdparty/build-definitions.sh File thirdparty/build-definitions.sh: http://gerrit.cloudera.org:8080/#/c/17554/16/thirdparty/build-definitions.sh@1101 PS16, Line 1101: cmake \ : -DCMAKE_BUILD_TYPE=release \ : -DCMAKE_INSTALL_PREFIX=$PREFIX \ : -DBUILD_SHARED_LIBS=OFF \ : -DOATPP_DISABLE_ENV_OBJECT_COUNTERS=ON \ : -DOATPP_BUILD_TESTS=OFF \ : $OATPP_SOURCE style nit: for line continuations, add an extra indent (see other build_xxx() functions in this file for reference) http://gerrit.cloudera.org:8080/#/c/17554/16/thirdparty/build-definitions.sh@1117 PS16, Line 1117: cmake \ : -DCMAKE_BUILD_TYPE=release \ : -DCMAKE_INSTALL_PREFIX=$PREFIX \ : -DBUILD_SHARED_LIBS=OFF \ : -DOATPP_BUILD_TESTS=OFF \ : -DOATPP_INSTALL=ON \ : $OATPP_SWAGGER_SOURCE ditto -- 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: 16 Gerrit-Owner: Khazar Mammadli <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Sat, 26 Jun 2021 00:20:20 +0000 Gerrit-HasComments: Yes
