Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/24358 )
Change subject: IMPALA-12955: Slim down profile tool deps ...................................................................... Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/24358/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/24358/3//COMMIT_MSG@16 PS3, Line 16: needs for profile archive parsing and printing. This technique of only defining some methods of a class seems like it could lead to some confusion with link failures later, but I'm ok dealing with that if it comes up. The alternative would be to move them to completely separate classes, which seems like a substantial lift. http://gerrit.cloudera.org:8080/#/c/24358/3/be/generated-sources/gen-cpp/CMakeLists.txt File be/generated-sources/gen-cpp/CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/24358/3/be/generated-sources/gen-cpp/CMakeLists.txt@83 PS3, Line 83: set(PROFILE_TOOL_SRC_FILES I'd lean towards making SRC_FILES include $PROFILE_TOOL_SRC_FILES instead of duplicating the list. Makes it easier if we need to add a new Thrift file to both. http://gerrit.cloudera.org:8080/#/c/24358/3/be/src/common/status-core.cc File be/src/common/status-core.cc: http://gerrit.cloudera.org:8080/#/c/24358/3/be/src/common/status-core.cc@31 PS3, Line 31: // TODO: is there a more controlled way to do this. I can't think of one at the moment. http://gerrit.cloudera.org:8080/#/c/24358/3/be/src/util/CMakeLists.txt File be/src/util/CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/24358/3/be/src/util/CMakeLists.txt@161 PS3, Line 161: set(PROFILE_TOOL_UTIL_SRCS Same comment about duplicating source lists. -- To view, visit http://gerrit.cloudera.org:8080/24358 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id476411dcf6c46079e2e2addc5cdd552bc23f8a1 Gerrit-Change-Number: 24358 Gerrit-PatchSet: 3 Gerrit-Owner: Aleksandr Efimov <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Comment-Date: Wed, 27 May 2026 17:15:27 +0000 Gerrit-HasComments: Yes
