Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21619 )
Change subject: [tools] Update plan-graph.py ...................................................................... Patch Set 2: Code-Review+1 (7 comments) http://gerrit.cloudera.org:8080/#/c/21619/1/bin/diagnostics/experimental/plan-graph.py File bin/diagnostics/experimental/plan-graph.py: http://gerrit.cloudera.org:8080/#/c/21619/1/bin/diagnostics/experimental/plan-graph.py@a155 PS1, Line 155: > Removing the present tense doesn't read as an improvement to me. Done http://gerrit.cloudera.org:8080/#/c/21619/1/bin/diagnostics/experimental/plan-graph.py@a287 PS1, Line 287: > nit: I wouldn't remove the 's'. If I run the new sentence through a grammar Done http://gerrit.cloudera.org:8080/#/c/21619/1/bin/diagnostics/experimental/plan-graph.py@a431 PS1, Line 431: > I don't think any of the changes to this sentence are necessary. Done http://gerrit.cloudera.org:8080/#/c/21619/1/bin/diagnostics/experimental/plan-graph.py@29 PS1, Line 29: # This script reads Impala plain text query profile and outputs GraphViz DOT file > Why the change in grammar? What was there before seemed more readable. Done http://gerrit.cloudera.org:8080/#/c/21619/1/bin/diagnostics/experimental/plan-graph.py@89 PS1, Line 89: ParseTask(Task.PARSE_FILTER_TABLE, > Why is this commented out? Done http://gerrit.cloudera.org:8080/#/c/21619/1/bin/diagnostics/experimental/plan-graph.py@470 PS1, Line 470: if next_line: > Can you add a comment on why you chose 50x? Done http://gerrit.cloudera.org:8080/#/c/21619/1/bin/diagnostics/experimental/plan-graph.py@490 PS1, Line 490: ori, dest, self.dict_to_dot_attrib(attrib))) > I'd suggest adding a helper method to set attrib['color'] when no_color is Done -- To view, visit http://gerrit.cloudera.org:8080/21619 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I22b0188bba3eef120c3e4b0f48408088123c4650 Gerrit-Change-Number: 21619 Gerrit-PatchSet: 2 Gerrit-Owner: Riza Suminto <[email protected]> Gerrit-Reviewer: Fang-Yu Rao <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Comment-Date: Wed, 31 Jul 2024 22:59:54 +0000 Gerrit-HasComments: Yes
