-
Notifications
You must be signed in to change notification settings - Fork 85
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 feature to support custom now_utc
implementations
#1397
Add feature to support custom now_utc
implementations
#1397
Conversation
not_utc
implementationsnow_utc
implementations
def22a2
to
bccee56
Compare
@itsyaasir @UMR1352: There will need to be some CI changes as I.e. testing will need to be split into I did not touch CI (yet). How would you want me to approach that issue? |
Yeah, I think splitting it will make sense in the CI |
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 for opening this PR, everything is looking fine to me 👍🏾 .
Let us get the CI passing now.
We will take care of CI 👌 |
Thanks a lot! |
@itsyaasir: In order to successfully compile Could we include that commit in this PR as well, or would you rather have that separately? |
You can add it in this PR, no problem |
@frederikrothenberger could you try to re-sign your commits? 🙏 |
This PR adds a feature to `identity_core` to allow specifying a custom function to get the current time (`Timestamp::now_utc`). The feature is disabled by default. Closes iotaledger#1391.
Also removes the unused dependency on `iota-crypto` (which also had a dependency on `js-sys` through the `random` feature).
4670be8
to
989e6ca
Compare
@eike-hass: The commits are now signed 👍 |
@frederikrothenberger Thanks for the contribution. |
In iotaledger#1397 my intention was to make the `js-sys` dependency mutually exclusive with the feature `custom_time`. As it turns out, it's not that simple and mixing target specific dependencies with feature specific dependencies does not actually work. See [here](https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#platform-specific-dependencies) and [here](https://doc.rust-lang.org/cargo/reference/features.html#mutually-exclusive-features). So this removes the broken feature reference in the dependency declaration and instead marks it as `optional`.
In iotaledger#1397 my intention was to make the `js-sys` dependency mutually exclusive with the feature `custom_time`. As it turns out, it's not that simple and mixing target specific dependencies with feature specific dependencies does not actually work. See [here](https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#platform-specific-dependencies) and [here](https://doc.rust-lang.org/cargo/reference/features.html#mutually-exclusive-features). So this removes the broken feature reference in the dependency declaration and instead marks it as `optional`. Also, the feature `custom_time` takes precedence over `js-sys` so these do not actually conflict and one _could_ enable both.
In iotaledger#1397 my intention was to make the `js-sys` dependency mutually exclusive with the feature `custom_time`. As it turns out, it's not that simple and mixing target specific dependencies with feature specific dependencies does not actually work. See [here](https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#platform-specific-dependencies) and [here](https://doc.rust-lang.org/cargo/reference/features.html#mutually-exclusive-features). So this removes the broken feature reference in the dependency declaration and instead marks it as `optional`. Also, the feature `custom_time` takes precedence over `js-sys` so these do not actually conflict and one _could_ enable both.
* Mark `js-sys` as optional for identity_core In #1397 my intention was to make the `js-sys` dependency mutually exclusive with the feature `custom_time`. As it turns out, it's not that simple and mixing target specific dependencies with feature specific dependencies does not actually work. See [here](https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#platform-specific-dependencies) and [here](https://doc.rust-lang.org/cargo/reference/features.html#mutually-exclusive-features). So this removes the broken feature reference in the dependency declaration and instead marks it as `optional`. Also, the feature `custom_time` takes precedence over `js-sys` so these do not actually conflict and one _could_ enable both. * Make js-sys a default feature * Fix defaults switch * Don't expose `js-sys` feature Co-authored-by: Yasir <yasirshariffa@gmail.com> --------- Co-authored-by: Yasir <yasirshariffa@gmail.com>
Description of change
This PR adds a feature to
identity_core
to allow specifying a custom function to get the current time (Timestamp::now_utc
). The feature is disabled by default.Fixes #1391.
Type of change
Add an
x
to the boxes that are relevant to your changes.How the change has been tested
Added an integration test to
identity_core
that uses the custom hook.Change checklist
Add an
x
to the boxes that are relevant to your changes.@itsyaasir: does this feature require more documentation?