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

Implement formatDateTime fractional second formatter #44060

Merged

Conversation

ltrk2
Copy link
Contributor

@ltrk2 ltrk2 commented Dec 8, 2022

Implements functionality requested in #33704.

Changelog category (leave one):

  • New Feature

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Implemented a fractional second formatter (%f) for formatDateTime.

@robot-ch-test-poll robot-ch-test-poll added the pr-feature Pull request with new product feature label Dec 8, 2022
@ltrk2 ltrk2 force-pushed the feature/format-datetime-fractional-second branch from 71ad470 to cae7a1d Compare December 8, 2022 22:53
@davenger
Copy link
Member

davenger commented Dec 8, 2022

Please add test cases were where fractional part starts with 0s (e.g. ".001") to make sure they are not lost.

@ltrk2
Copy link
Contributor Author

ltrk2 commented Dec 8, 2022

@davenger great call, thanks!

@davenger davenger added the can be tested Allows running workflows for external contributors label Dec 8, 2022
@davenger
Copy link
Member

davenger commented Dec 9, 2022

Function toFractionalSecond() seems confusing: looks like the caller cannot interpret its return value without knowing the precision of the original value. It might be better to always return nanoseconds regardless of the original precision and name it toNanosecond() or add precision parameter and convert to the specified precision.

@ltrk2
Copy link
Contributor Author

ltrk2 commented Dec 9, 2022

@davenger hmm.. that certainly is a valid point. I figured I'd just add it as a bonus. I'll drop that part for now, since I don't want to contribute something half-baked without a real use-case.

@davenger davenger self-assigned this Dec 11, 2022
@rschu1ze
Copy link
Member

@ltrk2 You will be interested in #43818 (which is almost merged). Do you think it is possible to implement the same format also for Joda style?

@ltrk2
Copy link
Contributor Author

ltrk2 commented Dec 12, 2022

@rschu1ze looking at the patch it certainly is a possibility for formatDateTimeInJodaSyntax. As for what fromUnixTimestampInJodaSyntax should do if it encountered a fractional second formatter could be a matter of debate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
can be tested Allows running workflows for external contributors pr-feature Pull request with new product feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants