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

add platform interface & refactor all Instant to Duration #9

Closed
wants to merge 7 commits into from
Closed

add platform interface & refactor all Instant to Duration #9

wants to merge 7 commits into from

Conversation

zhangzqs
Copy link
Contributor

No description provided.

@zhangzqs zhangzqs marked this pull request as draft May 19, 2024 09:51
@maxwase
Copy link
Owner

maxwase commented May 20, 2024

Thank you for the PR! I've looked into it, but didn't quite understand the idea behind it, here some of my thoughts:

  1. Usually there is no need for an abstraction to know which platform it runs on. Here I see how you propose to move the Instant type out of State, however later the crate still needs an Instant or it's analogue to compute the duration, which to me sounds like an extra step to get the elapsed value.
  2. I understand that you're proposing an abstraction that aims to allow user to choose how to implement elapsed, however IHMO the crate needs an Instant::now abstraction because elapsed is kind of fixed, it's always now - self, we just need a way to know the current time.

I'll open a PR shortly to implement the way I see it, a review from you would be appreciated as I don't really know your use case

@zhangzqs zhangzqs closed this by deleting the head repository Jul 5, 2024
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