Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/24358 )
Change subject: IMPALA-12955: Slim down profile tool deps ...................................................................... Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/24358/4/be/src/common/CMakeLists.txt File be/src/common/CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/24358/4/be/src/common/CMakeLists.txt@42 PS4, Line 42: # Keep impala-profile-tool away from Status methods that depend on debug/proto helpers. : add_library(ProfileToolCommon : status-core.cc : ) : add_dependencies(ProfileToolCommon gen-deps) As it is, status-core.cc is not a coherent thing with its own header file. If anyone changes existing code to use one of the other functions from status.h, impala-profile-tool will fail to link. There's no indication in status.h which functions are off limits and in which files. It's weird to restrict certain files to a subset of Status functionality. Another way to approach this is to split the GetStackTrace() functionality out of debug-util.cc into its own library with a header / cc file. Status can use that and keep most of its existing functionality. GetStackTrace() doesn't really require many dependencies by itself. Status's protobuf / Thrift conversions can move out to a different header / cc file. Unless I'm missing something, I think that gets us what we need. -- 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: 4 Gerrit-Owner: Aleksandr Efimov <[email protected]> Gerrit-Reviewer: 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: Fri, 29 May 2026 06:16:22 +0000 Gerrit-HasComments: Yes
