-
Notifications
You must be signed in to change notification settings - Fork 594
add wasm-bindgen conversion impls to Datetime #1750
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1750 +/- ##
==========================================
- Coverage 90.90% 90.90% -0.01%
==========================================
Files 39 39
Lines 16254 16254
==========================================
- Hits 14776 14775 -1
- Misses 1478 1479 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Closes #1749 |
|
Per the recommendations on BurntSushi/jiff#424, I just want to alert the maintainers of chrono that the module these traits live in (wasm-bindgen::convert) is unstable and internal. Implementing these traits could pose semver compatibility issues. |
| } | ||
| } | ||
|
|
||
| #[cfg(all( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To make this verbose, suggest wrapping these in a module, like so:
#[cfg(all(
target_arch = "wasm32",
feature = "wasmbind",
not(any(target_os = "emscripten", target_os = "wasi", target_os = "linux"))
))]
mod wasm_impls {
use wasm_bindgen::convert::{FromWasmAbi, ..};
impl .. { }
// etc
}|
(Please squash any changes from feedback into the original commit.) |
move wasm-bindgen trait impls into one module to reduce verbosity add datetime and utc imports to wasm-impls module as they were erroneously left out of the last commit
djc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
|
Ah, sorry, I missed your last comment on that. I think I'm still willing to take this, although I want an extra opt-in Cargo feature (let's call it |
|
Sounds good to me! This will be incredibly useful for me so I'm happy this is going through. |
This pull request adds the conversion
impls forwasm-bindgencrate's traits to enable usage in structs annotated with#[wasm_bindgen]