rusackas commented on PR #36315:
URL: https://github.com/apache/superset/pull/36315#issuecomment-3662512071
It seems there are still some issues that we might want to contend with:
1) Removal of 1Math.min1/`Math.max` — The original code handled backward
ranges gracefully. The new code assumes `start < stopExclusive`, which could
break if someone passes a negative range number or a Date before `d`.
2) Timezone inconsistency — `getMonthDomain` now uses UTC but
`getWeekDomain`, `getYearDomain` still use local time. CodeAnt flagged this
too. Could cause subtle mismatches.
3) Test assertion strengthening — Using `toContain('2025-03-01')` is
permissive. `'2025-03-01T23:00:00.000Z'` still passes that check even if
there's an off-by-hour bug.
4) No Python-side test coverage — The viz.py change has 0% patch coverage
per Codecov. Not the end of the world, but there might be an edge case. If
`start == end` and both are the first of a month, subtracting a day makes `end
< start`, potentially yielding a zero or negative range.
This is super close... if you're using some AI IDE, I'll bet it can bang out
these final touch-ups super quickly :D Again, thanks for your contribution and
your patience with this!
--
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]
