Wenzhe Zhou has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/19744 )

Change subject: IMPALA-11970: Optimized rendering for the query timeline display
......................................................................


Patch Set 5:

(14 comments)

http://gerrit.cloudera.org:8080/#/c/19744/5/www/query_timeline.tmpl
File www/query_timeline.tmpl:

http://gerrit.cloudera.org:8080/#/c/19744/5/www/query_timeline.tmpl@127
PS5, Line 127: collect
rename the function as collectFromProfile


http://gerrit.cloudera.org:8080/#/c/19744/5/www/query_timeline.tmpl@130
PS5, Line 130: maxts
should we define it as global variable?


http://gerrit.cloudera.org:8080/#/c/19744/5/www/query_timeline.tmpl@136
PS5, Line 136: // First pass: compute sizes
move this comment down after line 137


http://gerrit.cloudera.org:8080/#/c/19744/5/www/query_timeline.tmpl@137
PS5, Line 137:   try{
nit: add a space after try


http://gerrit.cloudera.org:8080/#/c/19744/5/www/query_timeline.tmpl@151
PS5, Line 151:  
nit: indent with 4 spaces.


http://gerrit.cloudera.org:8080/#/c/19744/5/www/query_timeline.tmpl@152
PS5, Line 152: )
nit: don't put it in a separate line.


http://gerrit.cloudera.org:8080/#/c/19744/5/www/query_timeline.tmpl@179
PS5, Line 179:               var table_name = pp.info_strings.find(({ key }) => 
key === "Table Name").value.split(/[.]/);
nit: wrap these two long lines


http://gerrit.cloudera.org:8080/#/c/19744/5/www/query_timeline.tmpl@204
PS5, Line 204:
nit: remove the empty line


http://gerrit.cloudera.org:8080/#/c/19744/5/www/query_timeline.tmpl@211
PS5, Line 211:
nit: indent in two spaces


http://gerrit.cloudera.org:8080/#/c/19744/5/www/query_timeline.tmpl@284
PS5, Line 284:   }catch(e) {
nit: add a space in front of catch


http://gerrit.cloudera.org:8080/#/c/19744/5/www/query_timeline.tmpl@363
PS5, Line 363:             timing_ctx.fillText("X", name_width + 
Math.min.apply(null, node.events[2].tslist) * px_per_ns,
nit: wrap these two long lines


http://gerrit.cloudera.org:8080/#/c/19744/5/www/query_timeline.tmpl@399
PS5, Line 399: ,
nit: add a space after ','


http://gerrit.cloudera.org:8080/#/c/19744/5/www/query_timeline.tmpl@471
PS5, Line 471: profile_available == false
change to: !profile_available


http://gerrit.cloudera.org:8080/#/c/19744/5/www/query_timeline.tmpl@474
PS5, Line 474: =>
nit: add space around =>, and wrap the long line



--
To view, visit http://gerrit.cloudera.org:8080/19744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iec458602266863b6e313f52b26a2dc35cff0db0f
Gerrit-Change-Number: 19744
Gerrit-PatchSet: 5
Gerrit-Owner: Surya Hebbar <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Kurt Deschler <[email protected]>
Gerrit-Reviewer: Surya Hebbar <[email protected]>
Gerrit-Reviewer: Wenzhe Zhou <[email protected]>
Gerrit-Comment-Date: Wed, 19 Apr 2023 20:29:02 +0000
Gerrit-HasComments: Yes

Reply via email to