socketry / async-websocket

Asynchronous WebSocket client and server, supporting HTTP/1 and HTTP/2 for Ruby.
MIT License
167 stars 18 forks source link

Add options for websocket-driver to README #7

Closed destructobeam closed 5 years ago

coveralls commented 5 years ago

Pull Request Test Coverage Report for Build 52


Totals Coverage Status
Change from base Build 47: 0.0%
Covered Lines: 72
Relevant Lines: 79

💛 - Coveralls
ioquatix commented 5 years ago

I am happy to do this, the only problem is that options = {} might not be compatible with future versions of ruby. It needs to be **options to expand the hash into options. The only thing I could suggest is actually using an option like headers: or something.

destructobeam commented 5 years ago

That's cool, probably easier to just take off the = {} in the actual method call. Will do that.

ioquatix commented 5 years ago

I would suggest removing , options = {} and then before that in the comment list valid options the user might want to pass?

destructobeam commented 5 years ago

Can still change to your comment above if you like, just thought having what most people will probably want to use in the example might make sense?

Do we want to list all possible websocket-driver options? Might be better to have a link to their docs for that?

ioquatix commented 5 years ago

I think it's a good idea to list commonly used options.

For Ruby,

    Async::WebSocket::Server.open(env, { protocols: %w[ws] }) do |connection|

is probably better as:

    Async::WebSocket::Server.open(env, protocols: ['ws']) do |connection|

Because https://github.com/socketry/async-websocket/blob/master/lib/async/websocket/server.rb#L26 is taking **options which in Ruby 3.0 will no longer be a hash.

destructobeam commented 5 years ago

Ahh okay, makes sense, I didn't see that had changed to a spread. Pushing an update.

ioquatix commented 5 years ago

Thanks for your effort and handling all feedback.

destructobeam commented 5 years ago

No worries! Thanks!

ioquatix commented 1 year ago

@destructobeam hello again, would you be willing to add your name to the .mailmap so I can update the copyright and license to accurately credit contributors? Thanks!