Skip to content
This repository has been archived by the owner on Aug 23, 2019. It is now read-only.

[WIP] More tests #5

Merged
merged 4 commits into from
Aug 1, 2015
Merged

[WIP] More tests #5

merged 4 commits into from
Aug 1, 2015

Conversation

dignifiedquire
Copy link
Member

Very slowly I'm getting to know the code base. Hopefully more interesting tests coming soon.

@daviddias
Copy link
Member

👍 Awesome! Thank you @dignifiedquire , want me to merge it now or want to add more tests to this PR?

@dignifiedquire
Copy link
Member Author

@diasdavid found my first bug while adding tests :)
If you want you can merge it, and I'll just do new PRs with more tests

@dignifiedquire
Copy link
Member Author

Needs libp2p/js-spdy-stream-muxer#3 to pass the tests

@@ -20,7 +20,7 @@ function Swarm () {

self.port = parseInt(process.env.IPFS_SWARM_PORT, 10) || 4001
self.connections = {} // {peerIdB58: {conn: <>, socket: <>}
self.handles = []
self.handles = {}
Copy link
Member

Choose a reason for hiding this comment

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

Any specific reason on having handles as an object instead of an array? I see you changed the looped and both work fine. I might just be missing a perf improv, perhaps?

Copy link
Member Author

Choose a reason for hiding this comment

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

No perf reason but the lookup you were trying to do if there was alread a handler wasn't working, so I changed it to an object, as otherwise we would have to search the whole list to check if we already have a handler.

daviddias added a commit that referenced this pull request Aug 1, 2015
@daviddias daviddias merged commit 16e9696 into libp2p:master Aug 1, 2015
@daviddias
Copy link
Member

Thank you @dignifiedquire, this looking very good! I've went ahead and merged this PR, so that you don't have to be "linking" the patched version of spdy-stream-muxer.

Plus is awesome to have bugs fixes pushed :) That was ⭐️

@dignifiedquire
Copy link
Member Author

Thanks happy to help, even though I still don't understand all the dark connection magic.

Btw you need to turn on travis, otherwise that badge will stay gray ;)

@daviddias daviddias mentioned this pull request Aug 3, 2015
35 tasks
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.

2 participants