innovark37 commented on PR #39330: URL: https://github.com/apache/superset/pull/39330#issuecomment-4478736081
@rusackas Thanks for the detailed review. I went through the items and addressed the actionable ones: - Locale normalization now replaces all underscores with hyphens using `replace(/_/g, '-')`, with a regression test for `zh_Hans_CN`. - `TaskList` and `TaskStatusIcon` now use the app root Redux state type from `src/views/store` instead of `ExplorePageState`. - `formatDuration` now reuses a module-level `Intl.DurationFormat` cache keyed by locale instead of creating a formatter on every call. - I also found and fixed a related negative-duration edge case in `parseMilliseconds`: the days-to-years conversion now uses `Math.trunc()` instead of `Math.floor()`, with a regression test for negative day durations. On the negative duration point: I double-checked the current parse-ms implementation, and it does not use `Math.abs()`; it preserves the sign via `Math.trunc()` for each duration field. The existing `createDurationFormatter` tests cover negative values like `-1000ms` and `-0.5s`, so I did not add manual sign-prefixing logic. On the sub-second truncation point: the default behavior is intentional. This change is meant to move duration formatting to the `Intl.DurationFormat` model rather than exactly emulate `pretty-ms` output. By default we omit sub-second fields, so `10500ms` formats as `10s`. Callers that need sub-second precision can opt in with `formatSubMilliseconds: true`. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
