-
Notifications
You must be signed in to change notification settings - Fork 18
[WIP] pull-streams #19
Conversation
@@ -2,7 +2,7 @@ | |||
"name": "multistream-select", | |||
"version": "0.10.0", | |||
"description": "JavaScript implementation of the multistream spec", | |||
"main": "lib/index.js", | |||
"main": "src/index.js", |
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.
- change before merge :)
Always impressive to see how pull-streams make everything more elegant and smaller |
handlersMap[key](new Connection(shake.rest(), rawConn)) | ||
} else { | ||
log('unkown protocol: %s', protocol) | ||
defaultHandler(protocol, shake.rest()) |
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.
what is the defaultHandler?
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 above, it's a function passed in, allowing to add special handling of non existent protocols.
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.
That is not part of the spec. If the protocol doesn't exist, na
is returned.
Last things to do before merge:
Good to have:
|
expect( | ||
data | ||
).to.be.eql( | ||
['banana'] |
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.
🍌 👌🏽
b89374e
to
89e5f07
Compare
- add test to verify that both dialer and listener send multistream handshake right away - fix structure and func naming - not wait on listener - support multiple selects
Node.js 4 and 5 don't like:
|
unnsupported feature
another one bites the dust! |
Tests are passing but needs clean up and integration verification