Skip to content
This repository was archived by the owner on Aug 24, 2021. It is now read-only.

feat: updates multihash table #76

Merged
merged 2 commits into from
Apr 21, 2020
Merged

feat: updates multihash table #76

merged 2 commits into from
Apr 21, 2020

Conversation

hugomrdias
Copy link
Member

@hugomrdias hugomrdias commented Apr 6, 2020

  • adds script to auto update
  • reduces bundle size by having only one explicit table ( 3kb gziped diff )
  • remove defaultLengths seems to not be used and multicodec tables doesn't have it

closes #73

BREAKING CHANGE: defaultLengths removed, skein is now lowercase

- adds script to auto update
- reduces bundle size by having only one explicit table
- remove defaultLengths seems to not be used and multicoded tables doesn't have it

BREAKING CHANGE: defaultLengths removed
@hugomrdias hugomrdias requested review from hacdias, vmx and jacobheun April 6, 2020 18:06
Copy link
Member

@vmx vmx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @hugomrdias!

@rvagg
Copy link
Member

rvagg commented Apr 7, 2020

also breaks by changing case of some of the names, like Skein -> skein

but very good change though! I wondered why this was manually replicated and I guess that's just historical.

@hugomrdias hugomrdias requested a review from achingbrain April 7, 2020 11:46
Copy link
Member

@hacdias hacdias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@vmx
Copy link
Member

vmx commented Apr 8, 2020

@hugomrdias could you please force-push this commit again with an updated commit message that also makes the "Skein" casing change as breaking change (this way it should show up nicely in the changelog). It could be done while merging, but i also forgot doing those things quite often :)

@hugomrdias
Copy link
Member Author

@jacobheun FYI changed the maintainer to me

@hugomrdias hugomrdias requested a review from jacobheun April 20, 2020 19:52
Copy link
Contributor

@jacobheun jacobheun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@hugomrdias hugomrdias merged commit 852ce6b into master Apr 21, 2020
@hugomrdias hugomrdias deleted the feat/update-table branch April 21, 2020 11:57
@achingbrain
Copy link
Member

This contains a breaking change, it should have gone out as v0.5.0.

ipld-git is currently broken as it depends on ~0.4.14 of this module and accesses the defaultLengths property from /src/constants.

Please can you re-release 0.4.19 as 0.4.21 then release 0.4.20 as 0.5.0.

achingbrain added a commit to ipld/js-ipld-git that referenced this pull request Jun 19, 2020
Since multiformats/js-multihash#76 was released
the `multihashes` module does not export default hash lengths any more.

Sadly it went out as a patch release which means `ipld-git` is broken.

This PR hard codes the hash length as it is known, and also removes
the `multihashes` dep as we can use the one exported from `multihashing-async`.

Ref: multiformats/js-multihash#76 (comment)
achingbrain added a commit to ipld/js-ipld-git that referenced this pull request Jun 19, 2020
Since multiformats/js-multihash#76 was released
the `multihashes` module does not export default hash lengths any more.

For reasons that are unclear it went out as a patch release which means
`ipld-git` is currently broken.

This PR hard codes the hash length as it is known, and also removes
the `multihashes` dep as we can use the one exported from `multihashing-async`
and updates other deps.

Ref: multiformats/js-multihash#76 (comment)
vmx pushed a commit to ipld/js-ipld-git that referenced this pull request Jun 19, 2020
Since multiformats/js-multihash#76 was released
the `multihashes` module does not export default hash lengths any more.

For reasons that are unclear it went out as a patch release which means
`ipld-git` is currently broken.

This PR hard codes the hash length as it is known, and also removes
the `multihashes` dep as we can use the one exported from `multihashing-async`
and updates other deps.

Ref: multiformats/js-multihash#76 (comment)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

remove constants.js size
6 participants