Skip to content

Check toMillis overflow in operators with Duration #1006

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

Closed
wants to merge 1 commit into from

Conversation

fvasco
Copy link
Contributor

@fvasco fvasco commented Feb 25, 2019

Fix #868

@qwwdfsad qwwdfsad self-requested a review February 26, 2019 12:31
@fvasco
Copy link
Contributor Author

fvasco commented Feb 26, 2019

Hi @qwwdfsad
I added some test, I you need some extra ones please notify me.

I have to confide you a problem, gradle test does not work on any my computers.
So I had tested all TimeTest methods manually, but unfortunately I does not know if they compile. Sorry for my miss.
Providing a battle tested docker image is a great help for me.

@fvasco
Copy link
Contributor Author

fvasco commented Feb 26, 2019

Please wait,
I wish to add some more tests and handle sub millisecond duration for all operators.

@fvasco
Copy link
Contributor Author

fvasco commented Feb 27, 2019

Hi @qwwdfsad
I catch this opportunity to fix another issue, any positive duration less than 1ms is truncated to 0ms, so no delay happens and withTimeout crashes.
To fix this issue all strictly positive duration are coerced in 1..Long.MAX_VALUE milliseconds.
What do you think?

@qwwdfsad
Copy link
Member

qwwdfsad commented Mar 6, 2019

I think this is a good idea, thank you.
I've pushed your branch, improved documentation and tests and will merge it manually as soon as tests are passed.

Thank you for your contribution!

@qwwdfsad
Copy link
Member

qwwdfsad commented Mar 6, 2019

Merged manually

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants