This repository has been archived by the owner on Feb 12, 2024. 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: make IPFS API static (remove api-manager) #3365
chore: make IPFS API static (remove api-manager) #3365
Changes from 19 commits
084b213
8e1d141
ac85e55
90f18fc
015fcfc
d321353
e42b9d7
877c620
f8dc797
e757d58
417abf7
8e63ab4
7816a7e
7c1bdac
8b386cb
caca4b9
f5e0e85
4fbd20b
fa7c2c1
a911154
b242530
9064688
4b6d668
eb66f38
d2c013d
26e4b80
a857bb7
8d736ab
7fe8d76
dcef0d3
e67525f
b3d0e23
d1e3e02
bec98c2
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
This file was deleted.
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.
See docs comment, peer IDs should be strings consistent with
ipfs.swarm.peers
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.
Current implementation is not and docs / types were incorrect. And I have submitted #3389 to address that separately.
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.
can we have a
PeerIdLike
exported from thepeerId
package ?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 think better alternative would be accept
PeerId
here and let the user of the API do the conversion. That said if we were to add a type topeer-id
I think it should beToPeerId
and there should bePeerId.from(thing:ToPeerId):PeerId
function.I think we already have couple of places with
ToThing
pattern.Either way can we deal with trickling types into separate packages as a followup work ?
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.
Created a followup here libp2p/js-peer-id#133
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 already have several
createFrom*
functions inpeer-id
, but they are targeted to a specific type https://github.com/libp2p/js-peer-id#importI think this would be nice to have, I would just go with
PeerId.createFrom(param: ToPeerId)
to be consistent with what we already have.