8000 Remove dependency on `identity_core` default features by frederikrothenberger · Pull Request #1408 · iotaledger/identity · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Remove dependency on identity_core default features #1408

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Sep 13, 2024
Merged

Remove dependency on identity_core default features #1408

merged 4 commits into from
Sep 13, 2024

Conversation

frederikrothenberger
Copy link

Description of change

identity_did and identity_document depend on identity_core but do not turn off default features. This means that these crates cannot be compiled for wasm32-unknown-unknown without a dependency on js-sys.

Given the default features are not required, removing them makes these crates compatible across a wider range of compilation targets.

Type of change

Add an x to the boxes that are relevant to your changes.

  • Bug fix (a non-breaking change which fixes an issue)
  • Enhancement (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Fix

How the change has been tested

Existing test suite.

Change checklist

Add an x to the boxes that are relevant to your changes.

  • I have followed the contribution guidelines for this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Remove dependency on identity_core default features
7b8676c 
`identity_did` and `identity_document` depend on `identity_core` but do
not turn off default features. This means that these crates cannot be
compiled for `wasm32-unknown-unknown` without a dependency on `js-sys`.

Given the default features are not required, removing them makes these
crates compatible across a wider range of compilation targets.
@frederikrothenberger frederikrothenberger requested a review from a team as a code owner September 4, 2024 12:52
@frederikrothenberger
Copy link
Author

@itsyaasir: I ran into another issue related to js-sys, this time because I wanted to make use of identity_credential, which depends on identity_did and indentity_document.

@itsyaasir
Copy link
Contributor

Hello! Thanks for doing this, I think this should be fine.

@itsyaasir itsyaasir added No changelog Excludes PR from becoming part of any changelog Rust Related to the core Rust code. Becomes part of the Rust changelog. labels Sep 9, 2024
@itsyaasir
Copy link
Contributor

@frederikrothenberger can you make sure everything is working well on your side before we merge this in ?

Thanks

@frederikrothenberger
Copy link
Author

@itsyaasir: There are other features that we need (e.g. #1406) but in terms of compiling the library (at least the crates that we currently use) such that it works on the ICP platform, I can confirm that it works.

Thanks.

@eike-hass eike-hass requested a review from a team September 10, 2024 10:19
@frederikrothenberger
Copy link
Author

@itsyaasir: Could this be merged?

@itsyaasir itsyaasir merged commit 13acb23 into iotaledger:main Sep 13, 2024
15 checks passed
@eike-hass eike-hass added Patch Change without affecting the API that requires a patch release. Part of "Patch" section in changelog and removed No changelog Excludes PR from becoming part of any changelog labels Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wulfraem wulfraem wulfraem approved these changes

@itsyaasir itsyaasir itsyaasir approved these changes

Assignees
No one assigned
Labels
Patch Change without affecting the API that requires a patch release. Part of "Patch" section in changelog Rust Related to the core Rust code. Becomes part of the Rust changelog.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@frederikrothenberger @itsyaasir @wulfraem @eike-hass
0