Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20294 )
Change subject: IMPALA-12288: Add BUILD_WITH_NO_TESTS option to remove test targets ...................................................................... Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/20294/5/be/src/catalog/CMakeLists.txt File be/src/catalog/CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/20294/5/be/src/catalog/CMakeLists.txt@29 PS5, Line 29: if (BUILD_WITH_NO_TESTS) > Hi yifan, I think it'd better to wrap the following code block within the ' Well, I think we can get the same result by returning early without modifying too much codes. Could you elaborate more on why is it recommended to wrap the codes into an if block? -- To view, visit http://gerrit.cloudera.org:8080/20294 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I575ce76176c9f6a05fd2db0f420ebe6926d0272a Gerrit-Change-Number: 20294 Gerrit-PatchSet: 5 Gerrit-Owner: Yifan Zhang <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Xiang Yang <[email protected]> Gerrit-Reviewer: Yifan Zhang <[email protected]> Gerrit-Comment-Date: Mon, 07 Aug 2023 06:38:57 +0000 Gerrit-HasComments: Yes
