-
Notifications
You must be signed in to change notification settings - Fork 39
fix: update to aegir 31 and others deps #141
Conversation
Thx for creating this PR. The latest update to multicodec breaks PeerId and a few other dependencies due to https://github.com/multiformats/js-cid/blob/master/package.json#L44 |
src/index.js
Outdated
@@ -3,7 +3,7 @@ | |||
const mh = require('multihashes') | |||
const multibase = require('multibase') | |||
const multicodec = require('multicodec') | |||
const { baseTable: codecs } = require('multicodec/src/base-table.js') | |||
const { baseTable: codecs } = require('multicodec/src/generated-table') |
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.
We should stop using non-public APIs. We broke the world once, I don't want it to break it again.
If there is no code to name mapping in js-multicodec, we should add it properly to the public API. I can have a look later this 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.
Cids exports code to name?
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.
AFAICT it only changed the file name nothing else
src/index.js
Outdated
@@ -3,7 +3,7 @@ | |||
const mh = require('multihashes') | |||
const multibase = require('multibase') | |||
const multicodec = require('multicodec') | |||
const { baseTable: codecs } = require('multicodec/src/base-table.js') | |||
const { nameToCode: codecs } = require('multicodec') |
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.
Having multicodec
imported twice is a bit strange, but I guess we want to move fast.
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.
Yay, now the public API of js-multicodec is used!
No description provided.