Laszlo Gaal has posted comments on this change. ( http://gerrit.cloudera.org:8080/22355 )
Change subject: Add aws-sdk-cpp (bedrock) libraries ...................................................................... Patch Set 4: (1 comment) Looks OK; just a minor style note about environment variable handling in buildall.sh http://gerrit.cloudera.org:8080/#/c/22355/4/buildall.sh File buildall.sh: http://gerrit.cloudera.org:8080/#/c/22355/4/buildall.sh@308 PS4, Line 308: export -n CURL_VERSION : export -n ZLIB_VERSION 'export -n' keeps the variable and its value defined, it just removes the "exportable" attribute from it; was this the original intention? If it's more about 'scoping' the version variables, I'd suggest using either of the above patterns (unfortunately buildall.sh is a bit inconsistent in this regard): - either use the inline form without the "export" keyword, like AWS_SDK_CPP_VERSION is used in L307 (and you could just use line continuation with trailing '\' characters to put different variables on different lines) - or use "unset" after the build.sh invocation to remove the variables completely, like in L301 above. -- To view, visit http://gerrit.cloudera.org:8080/22355 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c428195e8be24de53b2b30479a4966e43db74cd Gerrit-Change-Number: 22355 Gerrit-PatchSet: 4 Gerrit-Owner: Abhishek Rawat <[email protected]> Gerrit-Reviewer: Abhishek Rawat <[email protected]> Gerrit-Reviewer: Laszlo Gaal <[email protected]> Gerrit-Reviewer: Yida Wu <[email protected]> Gerrit-Comment-Date: Thu, 30 Jan 2025 20:37:45 +0000 Gerrit-HasComments: Yes
