Surya Hebbar 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 6: (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 Done http://gerrit.cloudera.org:8080/#/c/19744/5/www/query_timeline.tmpl@130 PS5, Line 130: maxts > should we define it as global variable? Yes, as it is also being used in rendering http://gerrit.cloudera.org:8080/#/c/19744/5/www/query_timeline.tmpl@136 PS5, Line 136: try { > move this comment down after line 137 Done http://gerrit.cloudera.org:8080/#/c/19744/5/www/query_timeline.tmpl@137 PS5, Line 137: // First pass: compute sizes > nit: add a space after try Done http://gerrit.cloudera.org:8080/#/c/19744/5/www/query_timeline.tmpl@151 PS5, Line 151: > nit: indent with 4 spaces. Done 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. Done http://gerrit.cloudera.org:8080/#/c/19744/5/www/query_timeline.tmpl@179 PS5, Line 179: .value.split(/[.]/); > nit: wrap these two long lines Done http://gerrit.cloudera.org:8080/#/c/19744/5/www/query_timeline.tmpl@204 PS5, Line 204: if (pp.event_sequences != undefined) { > nit: remove the empty line Done http://gerrit.cloudera.org:8080/#/c/19744/5/www/query_timeline.tmpl@211 PS5, Line 211: n > nit: indent in two spaces Done 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 Done http://gerrit.cloudera.org:8080/#/c/19744/5/www/query_timeline.tmpl@363 PS5, Line 363: timing_ctx.fillText("X", > nit: wrap these two long lines Done http://gerrit.cloudera.org:8080/#/c/19744/5/www/query_timeline.tmpl@399 PS5, Line 399: > nit: add a space after ',' Done http://gerrit.cloudera.org:8080/#/c/19744/5/www/query_timeline.tmpl@471 PS5, Line 471: ile = JSON.parse(req.respo > change to: !profile_available Done 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 Done -- 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: 6 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 21:40:15 +0000 Gerrit-HasComments: Yes
