shivaam commented on PR #64752: URL: https://github.com/apache/airflow/pull/64752#issuecomment-4190114985
## PR description doesn't match the actual diff The description claims five changes, but the diff only modifies **4 lines** in `utils.ts` (swapping `new Date().getTime()` → `dayjs().valueOf()`) plus a new test file. Three of the listed changes are not part of this PR — they already exist on `main`: > - Store raw ISO date strings from the API in the data array instead of pre-formatted display strings — formatting now only happens at display time (tooltips, tick labels) > - Skip tasks/groups where `start_date` is null (task hasn't started, no meaningful bar to show) > - Handle running groups by using current time as end date when `max_end_date` is null `transformGanttData` on `main` already: - Outputs ISO strings via `dayjs(...).toISOString()` (not pre-formatted display strings) - Filters out tasks with `null` `start_date` (`.filter((tryInstance) => tryInstance.start_date !== null)`) - Returns `undefined` for groups where `min_start_date === null || max_end_date === null` The only actual changes in this PR are: 1. Replacing `new Date(...).getTime()` with `dayjs(...).valueOf()` in the x-axis scale min/max calculations 2. Adding unit tests Could you update the description to accurately reflect what this PR changes? Taking credit for existing code makes it harder to review. Also noting the inconsistent null handling between the two scale blocks — in the `max` calculation you use `dayjs(item.x[1] ?? undefined)` but in the `min` calculation you use `dayjs(item.x[1])` without the fallback. This should be consistent. -- 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]
