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

Exclude DST consistently #40

Merged
merged 1 commit into from
Oct 6, 2023

Conversation

goncalossilva
Copy link
Member

Sparing Thomas from yet another review, could you help me with this one @lefcha? 😊

If source code finds a datetime without DST looking forward while
test code finds the a datetime without DST looking backward, the
UTC offset found may not be consistent.
@goncalossilva
Copy link
Member Author

This will unblock all other dependency update PRs.

Copy link
Contributor

@lefcha lefcha left a comment

Choose a reason for hiding this comment

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

Changes look good coding wise, I mean it seems we just moved some code to a separate function, in order to reuse it in testing; is it a refactor?

@goncalossilva
Copy link
Member Author

goncalossilva commented Oct 6, 2023

It's almost just a refactor... but it's a fix as well. The issue was that our runtime code was looking forward to find non-DST-affected datetime, while the tests were looking backward. This led to inconsistencies. The change is a refactor in the sense that now both use the same function, and a fix in the sense that both use the same approach.

@goncalossilva goncalossilva merged commit 6229ed6 into main Oct 6, 2023
2 checks passed
@goncalossilva goncalossilva deleted the goncalossilva/consistent-dst-exclusion branch October 6, 2023 21:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants