max-mapper / websocket-stream

websockets with the node stream API
BSD 2-Clause "Simplified" License
668 stars 114 forks source link

Binary with tests #96

Closed mcollina closed 8 years ago

mcollina commented 8 years ago

Extremely similar to #94, but improved to avoid that bad-looking option generation.

It also includes tests, and the README update.

cc @mafintosh @laszbalo @Introvertuous - please test before I release as minor.

mcollina commented 8 years ago

I am wondering if we should default binary to true, and always convert strings to Buffer.

Introvertuous commented 8 years ago

I originally assumed that the default was binary and was not sure what was wrong until I stumbled upon the aedes issue (61).

mcollina commented 8 years ago

@Introvertuous I have made the default.

@mafintosh I propose to release this as a minor release. There is a change of functionality, but I argue that was a bug.

mcollina commented 8 years ago

merging and releasing.