Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: remove peer-info from api #34

Merged
merged 2 commits into from
Apr 23, 2020
Merged

Conversation

vasco-santos
Copy link
Member

@vasco-santos vasco-santos commented Apr 10, 2020

In the context of deprecating peer-info as described on libp2p/js-libp2p#589, this PR removes the peer-info from being returned in the API, in favour of returning { id, addrs } in the same way as ipfs does.

BREAKING CHANGE: findProviders returns id and addrs properties instead of peer-info instance

Needs:

@vasco-santos vasco-santos force-pushed the chore/remove-peer-info-from-api branch 2 times, most recently from e475c19 to ec9ad61 Compare April 16, 2020 15:25
@vasco-santos vasco-santos marked this pull request as ready for review April 16, 2020 15:29
BREAKING CHANGE: findProviders returns id and addrs properties instead of peer-info instance
@vasco-santos vasco-santos force-pushed the chore/remove-peer-info-from-api branch from ec9ad61 to c7f7e2d Compare April 17, 2020 16:37
@vasco-santos vasco-santos requested a review from jacobheun April 17, 2020 16:43
src/index.js Outdated
@@ -63,7 +62,7 @@ class DelegatedContentRouting {
* @param {object} options
* @param {number} options.timeout How long the query can take. Defaults to 30 seconds
* @param {number} options.numProviders How many providers to find, defaults to 20
* @returns {AsyncIterable<PeerInfo>}
* @returns {AsyncIterable<{ id: PeerId, addrs: Multiaddr[] }>}
Copy link
Contributor

Choose a reason for hiding this comment

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

As mentioned in, libp2p/js-libp2p-delegated-peer-routing#25, I think we should make this return { id, multiaddrs } for api consistency.

@vasco-santos vasco-santos requested a review from jacobheun April 21, 2020 12:23
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

@jacobheun jacobheun merged commit 2c97221 into master Apr 23, 2020
@jacobheun jacobheun added this to the 0.5 milestone Apr 23, 2020
@jacobheun jacobheun deleted the chore/remove-peer-info-from-api branch April 23, 2020 12:15
@jacobheun
Copy link
Contributor

Included in beta release

dist-tags:
beta: 0.5.0    latest: 0.4.5  

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants