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

Update go-multihash. Now uses registry system. #118

Merged
merged 1 commit into from
Jun 26, 2021
Merged

Conversation

warpfork
Copy link
Member

@warpfork warpfork commented Mar 10, 2021

Some tests are updated to pull in all the hashes that they use, since some of them became optional rather than registered by default. This is what the new side-effecting imports handle.

Applications further downstream may require similar imports if they need to use hashes which are no longer registered by default. (These registrations only happen in tests in go-cid, so they're not passed on to downstreams; other applications and libraries must manage their own opt-in.)

You can't see it from here, but go-mulithash now uses registry system,
so it's reasonably possible to introduce new hashers, and to use
(some parts of!) go-multihash without bringing in lots of transitive
dependencies.

The main package of go-multihash still brings in everything
transitively that it did before, so go-cid's transitives aren't
shrinking, and no code is changing here... but it's closer.
If we did cut over to the new go-mulithash/core, we could make
many transitive dependencies become optional.

@warpfork
Copy link
Member Author

warpfork commented Mar 15, 2021

This turns out to be fairly inconsequential, because overall, go-cid still uses the go-multihash root package, which didn't significantly change in v0.0.15... so, downstreams of go-cid can already pull in the latest versions go-multihash, and things resolve just fine.

But we still might as well merge this update for the sake of propagating things proactively, afaict.

@warpfork warpfork requested a review from Stebalien March 15, 2021 18:26
Stebalien
Stebalien previously approved these changes Mar 15, 2021
@Stebalien Stebalien dismissed their stale review March 15, 2021 19:07

needs a release.

@Stebalien
Copy link
Member

This needs to depend on a released version of go-multihash, but otherwise SGTM.

You can't see it from here, but go-mulithash now uses registry system,
so it's reasonably possible to introduce new hashers, and to use
(some parts of!) go-multihash without bringing in lots of transitive
dependencies.

The main package of go-multihash still brings in everything
transitively that it did before, so go-cid's transitives aren't
shrinking, and no code is changing here... but it's closer.
If we did cut over to the new go-mulithash/core, we could make
many transitive dependencies become optional.
@Stebalien Stebalien merged commit 8e9280d into master Jun 26, 2021
@Stebalien Stebalien deleted the multihash-update branch June 26, 2021 00:23
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.

2 participants