Skip to content
This repository has been archived by the owner on Mar 10, 2020. It is now read-only.

attempt to handle changes to ipfs swarm peers api #427

Merged
merged 4 commits into from
Nov 23, 2016

Conversation

whyrusleeping
Copy link
Contributor

so obviously i don't javascript, but these tests failing my CI is getting old so i've made an effort.

Can someone who knows what they are doing help out here?

cc @diasdavid @dignifiedquire @victorbjelkholm

@dignifiedquire
Copy link
Contributor

thanks @whyrusleeping will take a look later today

@dignifiedquire dignifiedquire self-assigned this Nov 16, 2016
@dignifiedquire
Copy link
Contributor

Ready for review and merge

@dignifiedquire
Copy link
Contributor

Additional tests are in ipfs-inactive/interface-js-ipfs-core#94

@dignifiedquire dignifiedquire removed their assignment Nov 17, 2016
@daviddias
Copy link
Contributor

thank you @dignifiedquire

So, since we need to provide the same api for js-ipfs and js-ipfs-api and having two return types is weird, the best is just to level up js-ipfs to the latest go-ipfs swarm peers API and make js-ipfs-api always return the same thing, of course that with the caveat that when contacting a go-ipfs 0.4.4 node, some of the properties will be undefined.

@dignifiedquire
Copy link
Contributor

@diasdavid updated both PRs.

@daviddias
Copy link
Contributor

👍 now just needs an update on js-ipfs ipfs.swarm.peers

@dignifiedquire
Copy link
Contributor

@diasdavid js-ipfs PR ipfs/js-ipfs#600

Copy link
Contributor

@daviddias daviddias left a comment

Choose a reason for hiding this comment

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

Tests pass locally, everything LG

@dignifiedquire
Copy link
Contributor

Tests pass locally, everything LG

@diasdavid can we ship this?

@haadcode
Copy link
Contributor

Ship it! :)

@daviddias daviddias merged commit ef54889 into master Nov 23, 2016
@daviddias daviddias deleted the try-to-fix-swarm-peers branch November 23, 2016 12:42
@daviddias daviddias mentioned this pull request Nov 23, 2016
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.

4 participants