Skip to content
This repository has been archived by the owner on Oct 17, 2024. It is now read-only.

Implement RestartableTimer.tick #89

Merged
merged 2 commits into from
Jul 17, 2019
Merged

Implement RestartableTimer.tick #89

merged 2 commits into from
Jul 17, 2019

Conversation

natebosch
Copy link
Contributor

@natebosch natebosch commented Jul 16, 2019

Closes dart-lang/core#323

Extra cleanup:

  • change a constructor body to an initializer list
  • remove an @override annotation

@natebosch natebosch requested a review from srawlins July 16, 2019 21:01
Copy link
Contributor

@srawlins srawlins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent. I was wondering if this would be the contract; works for me!

@natebosch natebosch requested a review from lrhn July 16, 2019 23:26
@natebosch
Copy link
Contributor Author

Adding @lrhn in case there are any concerns since we're breaking the monotonic increase expectations. I don't think it's worth trying to do something more complex, though we could consider an implementation that keeps that guaranteed by keeping track of the tick when we reset() and adding to the new timer's tick..

@lrhn
Copy link
Contributor

lrhn commented Jul 17, 2019

No concerns. A reset timer works like a new timer, and the tick of a non-periodic timer is always 0 (before firing) or 1 (after firing) anyway.

/// recent countdown.
///
/// Calls to [reset] will also reset the tick so subsequent tick values may
/// not be strictly larger than previous values.
@override
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do remove the @override while you are here. We don't use @override in package:async. (This is the only instance of @override in the entire package).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I can for now.

I think we should strongly consider adding them everywhere in this package and enabling the lint. We implement a lot of interfaces and it's really nice to be able to see quickly which are the new methods.

@natebosch natebosch merged commit 5f2aece into master Jul 17, 2019
@natebosch natebosch deleted the implement-tick branch July 17, 2019 21:13
mosuem pushed a commit to dart-lang/core that referenced this pull request Oct 14, 2024
Closes dart-lang/async#88

Extra cleanup:
- change a constructor body to an initializer list
- remove an `@override` annotation
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging this pull request may close these issues.

RestartableTimer#tick not implemented
4 participants