Surya Hebbar has posted comments on this change. ( http://gerrit.cloudera.org:8080/19745 )
Change subject: IMPALA-11970: Shifting the timeline display to SVG ...................................................................... Patch Set 4: (15 comments) http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl File www/query_timeline.tmpl: http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@39 PS3, Line 39: <input type="checkbox" id="plan_order" onClick="renderTiming()"/> > No, it seems I missed it during the refactor. I will fix it now. Done http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@97 PS3, Line 97: return rect; > nit: indent two spaces Done http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@100 PS3, Line 100: fill_colo > nit: add space after comma Done http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@111 PS3, Line 111: ! > nit: add space around != Done http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@148 PS3, Line 148: svg.appendChild(get_svg_rect(phases[color_idx].color, x + 1, y + 1, > The -2 is deliberate to ensure that there is always a black line or outline Done http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@157 PS3, Line 157: var ignore_px = 2; // Don't print tiny skews > nit: add space in front of { Done http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@158 PS3, Line 158: if ( > nit: indent two spaces Done http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@156 PS3, Line 156: var dx = (endts - ts) * px_per_ns; : var ignore_px = 2; // Don't print tiny skews : if (Math.abs(dx) > ignore_px) { : svg.appendChild(get_svg_line("#505050", last_end - dx, y , last_end - dx, : > I will incrporate the same changes here. Done http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@330 PS3, Line 330: function renderTiming(ignored_arg) { > spaces around = Done http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@337 PS3, Line 337: > nit: add space around "=" in this line and following two lines. Done http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@363 PS3, Line 363: > nit: add space after , Done http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@428 PS3, Line 428: > nit: don't split as separate line Done http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@444 PS3, Line 444: > nit don't split as separate line Done http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@494 PS3, Line 494: r_tic).t > nit: add spaces around + Done http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@497 PS3, Line 497: > nit: add space around * Done -- To view, visit http://gerrit.cloudera.org:8080/19745 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b Gerrit-Change-Number: 19745 Gerrit-PatchSet: 4 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: Tue, 02 May 2023 09:47:03 +0000 Gerrit-HasComments: Yes
