Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/16855 )
Change subject: IMPALA-10390: impala-profile-tool JSON output ...................................................................... Patch Set 3: (1 comment) This looks good to me. I had a couple style nits. http://gerrit.cloudera.org:8080/#/c/16855/3/be/src/util/runtime-profile.h File be/src/util/runtime-profile.h: http://gerrit.cloudera.org:8080/#/c/16855/3/be/src/util/runtime-profile.h@647 PS3, Line 647: JsonProfileToStringProfile Nit: Should we combine JsonProfileToStringProfile and JsonProfileToPrettyStringProfile into one function that takes an additional boolean "pretty"? Separately, we might consider cutting off the final "Profile" part of this name so it is JsonProfileToString? -- To view, visit http://gerrit.cloudera.org:8080/16855 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I82ae0fe9379b7e3cbe93166adaa4c37212ea0f67 Gerrit-Change-Number: 16855 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Sat, 12 Dec 2020 01:16:20 +0000 Gerrit-HasComments: Yes
