Aleksandr Efimov has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/24358 )

Change subject: IMPALA-12955: Slim down profile tool deps
......................................................................


Patch Set 7:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/24358/7/be/src/common/CMakeLists.txt
File be/src/common/CMakeLists.txt:

http://gerrit.cloudera.org:8080/#/c/24358/7/be/src/common/CMakeLists.txt@33
PS7, Line 33: add_dependencies(StackTraceUtil gen-deps)
> Nit: I would prefer to put the new stack-trace.* files and library in be/sr
Done in PS8. Moved stack-trace.* and StackTraceUtil to be/src/util and made 
Util link StackTraceUtil with target_link_libraries(Util PUBLIC StackTraceUtil).


http://gerrit.cloudera.org:8080/#/c/24358/7/be/src/common/CMakeLists.txt@39
PS7, Line 39:   status.cc
> Nit: Rather than including status this way (and adding StackTraceUtil below
Done in PS8. Common now links StatusMinimal with target_link_libraries(Common 
PUBLIC StatusMinimal).


http://gerrit.cloudera.org:8080/#/c/24358/7/be/src/common/CMakeLists.txt@54
PS7, Line 54: add_dependencies(ProfileToolCommon gen-deps)
> Nit: Let's call this StatusMinimal, as we're likely to use this in other bi
Done in PS8. Renamed the target to StatusMinimal and linked it to 
StackTraceUtil with target_link_libraries(StatusMinimal PRIVATE StackTraceUtil).


http://gerrit.cloudera.org:8080/#/c/24358/7/be/src/common/status.cc
File be/src/common/status.cc:

http://gerrit.cloudera.org:8080/#/c/24358/7/be/src/common/status.cc@212
PS7, Line 212: }
> Nit: We use a 90 character line length and the existing code is fine. Pleas
Done in PS8. Reverted the whitespace-only wrapping changes in status.cc.



--
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: 7
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: Sat, 06 Jun 2026 16:48:37 +0000
Gerrit-HasComments: Yes

Reply via email to