Skip to content
This repository has been archived by the owner on Sep 3, 2021. It is now read-only.

fix: update to aegir 31 and others deps #141

Merged
merged 4 commits into from
Mar 2, 2021
Merged

fix: update to aegir 31 and others deps #141

merged 4 commits into from
Mar 2, 2021

Conversation

hugomrdias
Copy link
Member

No description provided.

@hugomrdias hugomrdias requested a review from vmx March 2, 2021 12:48
@robertkiel
Copy link

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')
Copy link
Member

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.

Copy link
Member Author

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?

Copy link
Member Author

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')
Copy link
Member

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.

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.

Yay, now the public API of js-multicodec is used!

@vmx vmx merged commit bcf2ba2 into master Mar 2, 2021
@vmx vmx deleted the fix/aegir31 branch March 2, 2021 15:55
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.

3 participants