Surya Hebbar has posted comments on this change. ( http://gerrit.cloudera.org:8080/21851 )
Change subject: IMPALA-13389: Refactor webUI scripts to use ES6 syntax ...................................................................... Patch Set 15: (4 comments) Thank you for the review. I am sorry for missing this in diff. I should have split this into 2 changes. Due to the large number of files, it became hard to see the diffs, for reviewers also. I will make sure to review everything once again. http://gerrit.cloudera.org:8080/#/c/21851/14/www/scripts/query_timeline/fragment_diagram.js File www/scripts/query_timeline/fragment_diagram.js: http://gerrit.cloudera.org:8080/#/c/21851/14/www/scripts/query_timeline/fragment_diagram.js@201 PS14, Line 201: const LAST_TS_INDEX = events[last_e_index].ts_list.length - 1; > Can this be removed? Done. I am sorry for missing this, during patchset 2, I was experimenting on https://gerrit.cloudera.org/c/21593/ http://gerrit.cloudera.org:8080/#/c/21851/14/www/scripts/query_timeline/fragment_diagram.js@271 PS14, Line 271: : y = rownum * row_height; : if (events[LAST_E_INDEX].no_bar === undefined) { : // Plan node's top and bottom outlines : let top_edge_ts, bottom_edge_ts; : if (bucketed) { : top_edge_ts = events[LAST_E_INDEX].parts[0].max; : bottom_edge_ts = events[LAST_E_INDEX].parts[bucket_size - 1].max; : } else { : top_edge_ts = events[LAST_E_INDEX].ts_list[0]; : bottom_edge_ts = events[LAST_E_INDEX].ts_list[LAST_TS_INDEX]; : } : plan_node.appendChild(getSvgLine(stroke_fill_colors.black, xoffset, y, : xoffset + top_edge_ts * px_per_ns, y, false)); : plan_node.appendChild(getSvgLine(stroke_fill_colors.black, xoffset, y + bar_height, : xoffset + bottom_edge_ts * px_per_ns, y + bar_height, : false)); : } > Can this be removed? Done http://gerrit.cloudera.org:8080/#/c/21851/14/www/scripts/query_timeline/fragment_diagram.js@609 PS14, Line 609: > Can this be removed? Done http://gerrit.cloudera.org:8080/#/c/21851/14/www/scripts/query_timeline/global_members.js File www/scripts/query_timeline/global_members.js: http://gerrit.cloudera.org:8080/#/c/21851/14/www/scripts/query_timeline/global_members.js@22 PS14, Line 22: // Stroke width of fragment diagrams borders : export const BORDER_STROKE_WIDTH = 2; : // Default margin provided for fragment diagram's header and footer : export const MARGIN_HEADER_FOOTER = 5; > Is it appropriate to prefix all this constants with QUERY_TIMELINE_? Done. As these were within the query_timeline subdirectory, I have not prefixed them. But, I have added the comments here now. -- To view, visit http://gerrit.cloudera.org:8080/21851 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie38f2c642ede14956a2c6d551a58e42538204768 Gerrit-Change-Number: 21851 Gerrit-PatchSet: 15 Gerrit-Owner: Surya Hebbar <sheb...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Kurt Deschler <kdesc...@cloudera.com> Gerrit-Reviewer: Michael Smith <michael.sm...@cloudera.com> Gerrit-Reviewer: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Reviewer: Surya Hebbar <sheb...@cloudera.com> Gerrit-Comment-Date: Thu, 20 Mar 2025 14:51:25 +0000 Gerrit-HasComments: Yes
