-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
feat: add time to int converters #25147
feat: add time to int converters #25147
Conversation
Co-authored-by: Alex Boten <[email protected]>
Co-authored-by: Alex Boten <[email protected]>
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.
For the failing tests, it may help for expected
to be a date object that you can invoke the equivalent time.Duration
function on for the .Equals
comparison. I think hard-coding the numeric value is causing timezone problems.
Co-authored-by: Tyler Helmuth <[email protected]>
Co-authored-by: Tyler Helmuth <[email protected]>
Co-authored-by: Tyler Helmuth <[email protected]>
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.
@fchikwekwe looks like the time structs fixed the unit tests. Once the linter is handled we'll be good to go
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.
@evan-bradley can you take a look at this PR as well.
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.
Looks good to me. Thanks for adding these.
Description: Allows conversion of time to nanoseconds, microseconds, milliseconds or seconds
Link to tracking Issue:
Related to #24686
Testing: Unit tests for each converter
Documentation: