search

sockjs / sockjs-erlang

WebSocket emulation - Erlang server
http://sockjs.org
Other
266 stars 120 forks source link

Merged Pull Requests #45 & #46 #48

Closed dergraf closed 11 years ago

dergraf commented 11 years ago

The Pull Requests #45 and #46 are both essential to run sockjs-erlang with the current versions of Erlang and Cowboy. This might save you some work when merging all together.

hyperthunk commented 11 years ago

@dergraf - how can we possibly QA such a massive set of changes? I'd rather handle the two pull requests separately.

dergraf commented 11 years ago

@hyperthunk true, but both pr's were sitting there since quite some time and it looked like for some reasons they're not getting merged. maybe nobody has time to merge and test it, so that's what I did. If it's not needed anymore I am happy to close my pr.

hyperthunk commented 11 years ago

Argh, I don't want to be awkward about this, and I do appreciate that these commits have been sitting around forever. I was hoping to merge them today, but I've had too many other things on my plate to go through them properly. Also I've got to consider the impact they'll have on future versions of RabbitMQ, where we use them in our webstomp plugin (and haven't yet upgraded to the latest cowboy).

I'll have a discussion about this and see if it's possible to merge them in without causing us any headaches. The tests don't all pass for me either, but I suspect that's something to do with my environment being set up wrong.

dergraf commented 11 years ago

new sockjs-erlang tag could also be a solution. but great to know that you are working on that. thanks, let me close this one.

gebi commented 11 years ago

@hyperthunk from what i wrote about my old merge request, all sock-js unit-test run successfully, except those that can not run, because in newer cowboy releases support for older types of websocket connections got removed.

I've not tested this new merge request yet.

hyperthunk commented 11 years ago

@gebi - as with your previous pull request, there's so many commits that it'll take me the best part of an afternoon (if not longer) just to review them. Since I'm not a maintainer, I can't commit that kind of time right now and my day job (RabbitMQ) does care about sockjs-erlang, since we use it in some of our plugins, but we have other priorities to attend to first.

This work will get done eventually, but as per my comments in pull request #46, if you could size down the patch so that it only deals with cowboy 0.8 support and not a myriad of other things I know nothing about, that would speed things up a lot.

Cheers.

gebi commented 11 years ago

@hyperthunk i'll try to get the minimal patches necessary for cowboy 0.8.x support rebased onto upstream/master (is that right?) and resubmit it as a new pulll-request.

hyperthunk commented 11 years ago

That would be fantastic. Thanks!