Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix formatRFC3339: adjust formatting offset for timezones with minutes #1890

Conversation

piotrl
Copy link
Contributor

@piotrl piotrl commented Aug 1, 2020

…with minutes (e.g. India, Australia).

First of all, thanks for making such a modular library! Once we found the issue, we could fix it in minutes in production by forking single-function :)

  • Affected time-zones:
    • Australia,
    • East of Canada,
    • India and Sri Lanka,
    • Islands near by New Zealand
    • I think India is the most populated affected timezone
  • Caused formatting offset in expression like +5.5:30 instead +05:30) which makes invalid date in parseISO()
  • It was simple mistake in division (-330/60 caused 5.5 hours offset instead full hour)
  • I used toInteger as it already have condition for positive and negative offset, using Math.floot() or Math.ceil(). I see in some other functions
  • Add specs for interesting and regular timezones for this use case

…s (e.g. India, Australia)

* Affected time-zones: Australia, West of Canada, India and Sri Lanka, Islands near by New Zeland. I think India is the most populated affected timezone
* Caused formatting offset in expression like `+5.5:30` instead `+05:30`) which makes invalid date in `parseISO()`
* It was simple mistake in division (`-330/60` caused `5.5` hours offset instead full hour)
* I used `toInteger` as it already have condition for positive and negative offset, using `Math.floot()` or `Math.ceil()`. I see in some other functions
* Add specs for interesting and regular timezones for this use case
@piotrl piotrl changed the title Fix formatRFC3339: adjust formatting offset for timezones with minute… Fix formatRFC3339: adjust formatting offset for timezones with minutes Aug 1, 2020
Copy link
Member

@kossnocorp kossnocorp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much for such a considerate work, I appreciate it! And sorry about waiting, but you did good forking it locally, that's the way!

@kossnocorp kossnocorp merged commit d9f1fa7 into date-fns:master Aug 27, 2020
@piotrl piotrl deleted the bugfix/formatRFC3339-timezone-india-sri-lanka branch August 27, 2020 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants