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 dependency with web-time #1098

Merged
merged 1 commit into from
Apr 11, 2024

Conversation

DanGould
Copy link
Contributor

@DanGould DanGould commented Apr 1, 2024

web-time offers a number of advantages over the instant crate

  • Actively maintained
  • Stable 1.x API
  • Exact std::time replacement
  • Compatible with rustls pki-types web feature
  • Re-exports std::time when not targeting wasm32-unknown-unknown without pulling in any dependencies

Copy link
Collaborator

@benthecarman benthecarman left a comment

Choose a reason for hiding this comment

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

i'd rather use std::time when we aren't on wasm32. no need to use this dep in those instances

@DanGould
Copy link
Contributor Author

DanGould commented Apr 2, 2024

This crate is just an alias for std::time when we aren't on wasm32-unknown-unknown. This dep will not be used in those instances thanks to rust's dead code elimination.

See: https://github.com/daxpedda/web-time/blob/main/src/lib.rs

@benthecarman
Copy link
Collaborator

Yeah but then we need to trust that dep vs just doing less of 10 lines of code to handle it ourself

@DanGould DanGould force-pushed the web-time branch 2 times, most recently from 0535dca to b805c40 Compare April 3, 2024 00:13
web-time offers a number of advantages over the instant crate

- Actively maintained
- Stable 1.x API
- Exact std::time replacement
- Compatible with rustls pki-types web feature
- Re-exports std::time when not targeting wasm32-unknown-unknown
  without pulling in any dependencies
@DanGould DanGould requested a review from benthecarman April 3, 2024 00:57
Copy link
Collaborator

@benthecarman benthecarman left a comment

Choose a reason for hiding this comment

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

lgtm

@DanGould
Copy link
Contributor Author

DanGould commented Apr 3, 2024

Thanks for the prompt review

@TonyGiorgio TonyGiorgio merged commit a41f8f3 into MutinyWallet:master Apr 11, 2024
9 checks passed
@DanGould DanGould deleted the web-time branch April 11, 2024 04:20
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