This repository has been archived by the owner on Oct 1, 2021. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
chore: upgrade to new multiformats module #98
chore: upgrade to new multiformats module #98
Changes from 5 commits
f98d385
e22501b
17248ac
155bc90
d4d09e7
ef6c211
3546fb9
6c38340
9b5a552
c9d01ea
a2a7a59
a4ce9c6
90a0e78
f402825
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
const block = Block.encode({ value: pinRoot, codec: dagPb, hasher: sha256 })
is available to shorten this slightly, then you can pickblock.cid.bytes
out of it.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.
It looks like
Block.encode
will only ever give me a v1 CID back and we need a v0 CID here?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.
Could just pull the multihash bytes out of the v1 CID, I guess..
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.
mmm,
block.cid.multihash
should do it. Alternatives includeblock.cid.toV0().bytes
(which is more explicit but a little wasteful) or adding acidVersion
argument toBlock.encode
.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.
I don't think Block.encode will save that much, and probably it's best to not add support for v0 there if we eventually want to break from v0. I would personally leave it as is.
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.
This repo will always have to handle v0 as it needs to be able to migrate repos from before v1 was a thing.