geido commented on code in PR #31333: URL: https://github.com/apache/superset/pull/31333#discussion_r1941126511
########## superset-frontend/packages/superset-ui-core/src/query/getMetricLabel.ts: ########## @@ -19,17 +19,23 @@ import { QueryFormMetric, isSavedMetric, isAdhocMetricSimple } from './types'; -export default function getMetricLabel(metric: QueryFormMetric): string { +export default function getMetricLabel( + metric: QueryFormMetric, + index?: number, + queryFormMetrics?: QueryFormMetric[], + verboseMap: Record<string, string> = {}, Review Comment: Nit. Does it need to be defaulting to an empty object or can make this also optional and check for both undefined and empty? ########## superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts: ########## @@ -291,12 +292,20 @@ export default function transformProps( const showValueIndexesB = extractShowValueIndexes(rawSeriesB, { stack, }); + + const metricsLabels = metrics + .map(metric => getMetricLabel(metric, undefined, undefined, verboseMap)) Review Comment: Can we move the `verboseMap` before the other two optional parameters in the function so we don't have to pass the `undefined`s here at all? I don't see many other places mapping these values, so it might be safe to just revise the params positions, but if you feel that might introduce issues, feel free to skip this comment -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org