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

Timer: handle timeout correctly #42854

Merged
merged 1 commit into from
Nov 8, 2021
Merged

Timer: handle timeout correctly #42854

merged 1 commit into from
Nov 8, 2021

Conversation

vtjnash
Copy link
Member

@vtjnash vtjnash commented Oct 29, 2021

I am not sure why we ever used round+1 instead of ceil+1, as this is
strictly more correct.

I am not sure why we ever used round+1 instead of ceil+1, as this is
strictly more correct.
@vtjnash vtjnash added backport 1.6 Change should be backported to release-1.6 backport 1.7 labels Oct 29, 2021
@vtjnash vtjnash merged commit d6f59fa into master Nov 8, 2021
@vtjnash vtjnash deleted the jn/timer-ceil branch November 8, 2021 19:49
@IanButterworth
Copy link
Member

Could Timer support a reset(::Timer) method?

That way interval creep could be addressed by something like

Timer(0; interval = 5) do t
    foo()
    Dates.second(Dates.now()) % 5 != 0 && reset(t)
end

@vtjnash
Copy link
Member Author

vtjnash commented Nov 10, 2021

What does reset do?

@IanButterworth
Copy link
Member

Reset the timing as if the timer had just been recreated.

@IanButterworth
Copy link
Member

It could error if the timer is closed

KristofferC pushed a commit that referenced this pull request Nov 10, 2021
I am not sure why we ever used round+1 instead of ceil+1, as this is
simply strictly more correct.

(cherry picked from commit d6f59fa)
daviehh pushed a commit to daviehh/julia that referenced this pull request Nov 16, 2021
I am not sure why we ever used round+1 instead of ceil+1, as this is
simply strictly more correct.

(cherry picked from commit d6f59fa)
@KristofferC KristofferC mentioned this pull request Nov 19, 2021
29 tasks
KristofferC pushed a commit that referenced this pull request Dec 7, 2021
I am not sure why we ever used round+1 instead of ceil+1, as this is
simply strictly more correct.

(cherry picked from commit d6f59fa)
@KristofferC KristofferC removed the backport 1.6 Change should be backported to release-1.6 label Dec 11, 2021
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Feb 22, 2022
I am not sure why we ever used round+1 instead of ceil+1, as this is
simply strictly more correct.
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Mar 8, 2022
I am not sure why we ever used round+1 instead of ceil+1, as this is
simply strictly more correct.
staticfloat pushed a commit that referenced this pull request Dec 23, 2022
I am not sure why we ever used round+1 instead of ceil+1, as this is
simply strictly more correct.

(cherry picked from commit d6f59fa)
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.

3 participants