abhinav-phi commented on PR #44212: URL: https://github.com/apache/superset/pull/44212#issuecomment-5685903891
@rusackas good catch — you're right, they have exactly the same problem, and there's no reason to scope the year to day/week. My first pass was just matching the shape of the report (a daily-granularity scatter), not a deliberate design choice. I've extended `%Y` to the `hour`, `minute` and `second` tiers too: | tier | before | after | | --- | --- | --- | | second | `Thu Mar 20, 03:45:12 PM` | `Thu Mar 20 2016, 03:45:12 PM` | | minute | `Thu Mar 20, 03:45 PM` | `Thu Mar 20 2016, 03:45 PM` | | hour | `Thu Mar 20, 03 PM` | `Thu Mar 20 2016, 03 PM` | | day / week | `Thu Mar 20` | `Thu Mar 20 2016` | The year goes with the date part (`%a %b %d %Y, …`) so the date renders the same as the day/week tier, and the same-date-different-year case (`2014-03-20` vs `2026-03-20`) now produces different strings. Tests cover every tier in the default locale and a non-default one. Two things I left alone on purpose — happy to change either if you disagree: - `millisecond` (`.%L`, i.e. `.345`): it's the bare fractional-second fragment the multi-formatter emits for sub-second resolution, not a full timestamp, so a year there would read oddly. Making ms-precision tooltips self-describing is a separate fix, since it needs a real date+time prefix rather than `.345`. - `%d` stays zero-padded in the sub-day tiers (`Mar 03`) while day/week use `%-e` (`Mar 3`). Pre-existing, kept as-is to keep this diff focused. Unrelated to the review: `master` currently resolves to two Alembic heads (#43939 and #43490), which made every `flask db upgrade head` job fail on this PR's merge commit (playwright, cypress, docker-build (dev) and their `*-required` gates). I rebased onto current `master` and added the no-op merge revision joining both heads — the added file is byte-identical to the one in #44288, so whichever PR lands first the other still merges cleanly without creating a third head. `git merge-tree` of this branch against `master` now resolves to exactly one head (`e2f3a1b9c640`). If someone can approve the workflow runs for the new commit, everything should come back green. -- 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]
