GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||
GeezerVison is showing github.com — fix(utils): pick the timezone abbreviation from the UTC offset, not the formatted date by laoyyu · Pull Request #9923 · simstudioai/sim ConversationWhat
const isDSTObserved = januaryFormatter.format(januaryDate) !== julyFormatter.format(julyDate) ... const isDST = currentFormatter.format(date) !== januaryFormatter.format(januaryDate)
FixCompare the zone's UTC offsets at the two ends of the year instead of formatted strings (
How it was checked
Fixes #9831. Prepared with an AI coding assistant; I reviewed the diff and ran the checks above.
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
…he formatted date getTimezoneAbbreviation compared Intl.DateTimeFormat#format() results of two dates in the same year. The formatted string always contains the calendar date, so the comparison reported "DST is observed" for every zone and then "the target date is daylight time" for every date: Los Angeles in January came out as PDT, London as BST, Sydney in July as AEDT. Compare the zone's UTC offsets at the two ends of the year instead. A zone whose offsets match does not observe DST; otherwise the smaller offset is standard time and any larger offset on the target date is daylight time, which covers both hemispheres.
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
|
There was an error while loading. Please reload this page.
[Medium impact] The change reaches time-zone labels in date formatting and schedule summaries. The timezone fix appears sound, but the added comment must meet the repository's explicit rules before merging. Findings
Summary
Reviews (1) · Last reviewed commit: "fix(utils): pick the timezone abbreviati..." · Reviewed by Greptile |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| // Compare the zone's UTC offsets across the year: `format()` always | ||
| // includes the calendar date, so comparing formatted strings reported | ||
| // "DST observed" for the whole year and then "daylight" for every date. |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Comment records change history
The new comment recounts the previous bug, but CLAUDE.md says change history belongs in commit messages. Keep the explanation that format() includes the calendar date and remove the account of the old behavior. This repository requirement must be satisfied before merging.
| // Compare the zone's UTC offsets across the year: `format()` always | |
| // includes the calendar date, so comparing formatted strings reported | |
| // "DST observed" for the whole year and then "daylight" for every date. | |
| // Compare UTC offsets because `format()` also includes the calendar date. |
Context Used: CLAUDE.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
[BUG] Timezone formatter labels winter dates with daylight-time abbreviations
There was an error while loading. Please reload this page.