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

[time] Add isBetweenDates & isBetweenDateTimes polyfills to ZonedDateTime #203

Merged
merged 4 commits into from
Dec 23, 2022

Conversation

florian-h05
Copy link
Contributor

@florian-h05 florian-h05 commented Dec 23, 2022

Required for openhab/openhab-webui#1597.

This extends our polyfills for time.ZonedDateTime with isBetweenDates and isBetweenDateTimes to complete isBetweenTimes.

@digitaldan Please especially review f2d6982, the rest is just some reordering to allow some unit tests for toZDT.
@rkoshak I would also take a review of you, since you originally authored most parts or the time namespace.

The new methods and all polyfills in general are now testes by the unit tests.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
@florian-h05 florian-h05 added the enhancement New feature or request label Dec 23, 2022
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
@rkoshak
Copy link
Contributor

rkoshak commented Dec 23, 2022

LGTM. I'm not sure I understand the polyfills part and I need to get smart on the unit tests for my own good. But the actual functions look like I would have coded them. :-D

@florian-h05
Copy link
Contributor Author

Thanks, I will merge then 👍

@florian-h05 florian-h05 merged commit a235d1c into openhab:main Dec 23, 2022
@florian-h05 florian-h05 deleted the time-extend-polyfills branch December 23, 2022 22:34
@florian-h05
Copy link
Contributor Author

@rkoshak
With this, Stefan and I are nearly finished with porting the existing Blocks. The only ones that are left are two datetimeoffset Blocks and the persistence blocks, mainly because we were missing functionality on the time namespace.

@florian-h05 florian-h05 added this to the to be released milestone Dec 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants