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

Replace instant with web-time #666

Merged
merged 1 commit into from
Nov 11, 2024
Merged

Replace instant with web-time #666

merged 1 commit into from
Nov 11, 2024

Conversation

torokati44
Copy link
Contributor

@torokati44 torokati44 commented Nov 10, 2024

Fixes #665. See: https://crates.io/crates/web-time.

Note that unlike instant, web-time falls back to std::time on non-WASM, so this kind of conditional exports are not really necessary:

#[cfg(not(target_arch = "wasm32"))]
use std::time::Instant;
#[cfg(target_arch = "wasm32")]
use web_time::Instant;

Instead, it could unconditionally be web_time::Instant, if making web-time a (really light, to be fair) dependency on non-WASM platforms as well is acceptable. I can update the PR accordingly, if desired.

emilk added a commit to rerun-io/rerun that referenced this pull request Nov 11, 2024
* Fixes red main
* Real fix coming in console-rs/indicatif#666

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
Copy link
Member

@djc djc left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Instead, it could unconditionally be web_time::Instant, if making web-time a (really light, to be fair) dependency on non-WASM platforms as well is acceptable. I can update the PR accordingly, if desired.

I prefer keeping it like this.

@djc djc merged commit 884ddfb into console-rs:main Nov 11, 2024
10 checks passed
@torokati44
Copy link
Contributor Author

Thank you for the quick merge!
As with every PR - and especially ones motivated by issues downstream -, I'd highly appreciate a new release on crates.io as soon after merging as possible. ☺️

@djc djc mentioned this pull request Nov 11, 2024
@djc
Copy link
Member

djc commented Nov 11, 2024

@torokati44 see #667.

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.

The dependency instant is no longer maintained - consider switching to web-time instead
2 participants