Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18103 )
Change subject: [thirdparty] add jwt-cpp to thirdparty build ...................................................................... Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/18103/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/18103/1//COMMIT_MSG@9 PS1, Line 9: require > nit: required Done http://gerrit.cloudera.org:8080/#/c/18103/1/thirdparty/build-definitions.sh File thirdparty/build-definitions.sh: http://gerrit.cloudera.org:8080/#/c/18103/1/thirdparty/build-definitions.sh@1129 PS1, Line 1129: - > nit: use underscore Done http://gerrit.cloudera.org:8080/#/c/18103/1/thirdparty/build-definitions.sh@1138 PS1, Line 1138: "$OPENSSL_ROOT_DIR" \ > This looks a bit strange: why not to pass all what's necessary in this cont Done http://gerrit.cloudera.org:8080/#/c/18103/1/thirdparty/build-thirdparty.sh File thirdparty/build-thirdparty.sh: http://gerrit.cloudera.org:8080/#/c/18103/1/thirdparty/build-thirdparty.sh@181 PS1, Line 181: OPENSSL_ROOT_DIR="-DOPENSSL_ROOT_DIR=$homebrew_openssl_dir" > This isn't a good naming for a variable: at least, OPENSSL_ROOT_DIR is supp Done http://gerrit.cloudera.org:8080/#/c/18103/1/thirdparty/patches/jwt-cpp-openssl-1.0.1.patch File thirdparty/patches/jwt-cpp-openssl-1.0.1.patch: http://gerrit.cloudera.org:8080/#/c/18103/1/thirdparty/patches/jwt-cpp-openssl-1.0.1.patch@18 PS1, Line 18: +set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11") : + > It turns out this piece isn't necessary if building with C++11-cabaple comp Done -- To view, visit http://gerrit.cloudera.org:8080/18103 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idab887e1e0c44e9ebe6897defec8238c020cccbb Gerrit-Change-Number: 18103 Gerrit-PatchSet: 1 Gerrit-Owner: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 26 Jan 2022 21:25:35 +0000 Gerrit-HasComments: Yes
