-
Notifications
You must be signed in to change notification settings - Fork 16
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(core): Add getUTCMonday, fix tests in U.S. timezone #879
Conversation
Pull Request Test Coverage Report for Build 9505380995Details
💛 - Coveralls |
1 similar comment
Pull Request Test Coverage Report for Build 9505380995Details
💛 - Coveralls |
Visit the preview URL for this PR (updated for commit 0e81c14): https://cartodb-fb-storybook-react-dev--pr879-fix-timezone-agn-tzpdwud3.web.app (expires Fri, 21 Jun 2024 13:34:42 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 517cc4d31d7e09cf277774e034094b67c301cd4c |
Pull Request Test Coverage Report for Build 9505380995Details
💛 - Coveralls |
Pull Request Test Coverage Report for Build 9505453641Details
💛 - Coveralls |
2 similar comments
Pull Request Test Coverage Report for Build 9505453641Details
💛 - Coveralls |
Pull Request Test Coverage Report for Build 9505453641Details
💛 - Coveralls |
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.
Please, add an entry to the changelog on every PR 🙏
I added for you the last one that was missing, here: https://github.com/CartoDB/carto-react/pull/881/files#diff-06572a96a58dc510037d5efa622f9bec8519bc1beab13c9f251e97e657a9d4ed
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.
May the time be with you, now... 🕐
f960e9c
to
0e81c14
Compare
@vmilan changelog updated, thank you! |
Pull Request Test Coverage Report for Build 9516971163Details
💛 - Coveralls |
2 similar comments
Pull Request Test Coverage Report for Build 9516971163Details
💛 - Coveralls |
Pull Request Test Coverage Report for Build 9516971163Details
💛 - Coveralls |
Description
Currently the tests do not pass when running locally from a U.S. time zone, fixed in this PR. Existing code in
groupByDate.js
used UTC time for all groupings, andTimeSeriesWidget
uses local time, but both call intogetMonday
which cannot do both ... so I think it's likely that a UTC-specific version ofgetMonday
is required.Changes: