-
Notifications
You must be signed in to change notification settings - Fork 45
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
fix: don't depend on unnecessary multihash
features
#77
fix: don't depend on unnecessary multihash
features
#77
Conversation
multihash
featuresmultihash
features
@thomaseizinger do you have some time to look at the "Build" CI failures? I am taking a look at the code coverage failures now. |
Maybe later tonight yes, otherwise likely only next week. |
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.
Approved, once CI is fixed.
@vmx Can you change the repository settings as per https://matklad.github.io/2022/10/24/actions-permissions.html? The workflows don't run at the moment. |
Done |
All we need from the multihash is for it to be a data structure that we pass around. We only ever use the identity "hasher" and the sha256 hasher. Those are easily implemented without depending the (fairly heavy) machinery in `multihash`. Unfortunately, this patch by itself does not yet lighten our dependency tree because `multiaddr` activates those features unconditionally. I opened a companion PR for this: multiformats/rust-multiaddr#77. multiformats/rust-multiaddr#77 is another breaking change and we are trying to delay those at the moment. However, it will (hopefully) land eventually which should then be much easier to implement. Fixes #3276. Pull-Request: #3514.
With these features activated,
multihash
is quite heavy. Whilst we are in the process of fixing this, removing these features is a first good step towards that.I think technically, this is a breaking change unfortunately because we re-export all of
multihash
and thus someone could have relied on the presence ofmultihash::Code
through the re-export ofmultihash
.