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

Better Timestamp Parsing #2713

Merged
merged 2 commits into from
Aug 23, 2024

Conversation

sinistersnare
Copy link
Contributor

@sinistersnare sinistersnare commented Aug 16, 2024

Hello!

This PR improves the parsing of timestamp strings in Perspective! It also improves our Timezone parsing logic to better support strange timezones (like Nepal, Myanmar, or Mars). This PR also adds a few tests, so I hope that you have a good day and can sleep more restfully with this in mind.

Thanks!

@sinistersnare sinistersnare force-pushed the feature/better-timestamp-parsing branch from 414ec7f to 675272a Compare August 16, 2024 21:12
Signed-off-by: Davis Silverman <davis@thedav.is>
TZs were being handled backward by the parser, and now we also support
Timezones after the fractional second part.

Perspective still only supports milliseconds internally, so this only
parses them correctly, but truncates their values to millisecond precision.

Signed-off-by: Davis Silverman <davis@thedav.is>
@sinistersnare sinistersnare force-pushed the feature/better-timestamp-parsing branch from 675272a to bdabded Compare August 17, 2024 01:28
@sinistersnare sinistersnare marked this pull request as ready for review August 17, 2024 01:30
@texodus texodus added the enhancement Feature requests or improvements label Aug 23, 2024
Copy link
Member

@texodus texodus left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Looks good!

Outstanding test coverage!

@texodus texodus merged commit e179382 into finos:master Aug 23, 2024
9 checks passed
@sinistersnare sinistersnare deleted the feature/better-timestamp-parsing branch August 23, 2024 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature requests or improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants