-
Notifications
You must be signed in to change notification settings - Fork 465
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 Daylight Savings parsing #396
base: master
Are you sure you want to change the base?
Conversation
Previously the offset change for Daylight Savings was unnecessarily added all the time, resulting in this discrepancy when the local time was set to UTC-7: ``` Time.parse('2020-11-01 00:00:00') => 2020-11-01 00:00:00 -0700 Chronic.parse('2020-11-01 00:00:00') => 2020-11-01 01:00:00 -0700 ``` Each time we add an offset to a base time, we need to check whether this crosses Daylight Savings boundaries. Only if it does should we adjust the time by the offset. Closes mojombo#147
26c5c25
to
d143986
Compare
@leejarvis Are you still maintaining this gem? If so, would you mind reviewing and pushing out an update? This bug causes a number of issues on gems that depend on it. |
He haven't been checking this for years. My unpublished |
@@ -43,6 +43,10 @@ def test_handle_generic | |||
|
|||
assert_nil Chronic.parse("1/1/32.1") | |||
|
|||
time = Chronic.parse("2020-11-01 00:00:00") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This tests your PR only when system's timezone is one you have, I have different one so for me this test passes either way even without your PR. Need to write such test that would test your specific case with your timezone.
I'm afraid I no longer have time to maintain Chronic. I'm more than open to adding more contributors if anyone is willing though |
Previously the offset change for Daylight Savings was unnecessarily
added all the time, resulting in this discrepancy when the local time
was set to UTC-7:
Each time we add an offset to a base time, we need to check whether this
crosses Daylight Savings boundaries. Only if it does should we adjust
the time by the offset.
Closes #147