Skip to content

WebSocket enhancements#587

Merged
normanmaurer merged 1 commit intonetty:masterfrom
danbev:websocket-enhancement
Sep 9, 2012
Merged

WebSocket enhancements#587
normanmaurer merged 1 commit intonetty:masterfrom
danbev:websocket-enhancement

Conversation

@danbev
Copy link
Copy Markdown
Member

@danbev danbev commented Sep 9, 2012

  • Refactoring and adding suggestions from Norman and Vibul.

- Refactoring and adding suggestions from Norman and Vibul.
@ghost ghost assigned normanmaurer Sep 9, 2012
normanmaurer added a commit that referenced this pull request Sep 9, 2012
@normanmaurer normanmaurer merged commit 722c63d into netty:master Sep 9, 2012
@normanmaurer
Copy link
Copy Markdown
Member

@danbev you handled the future the right way. I think its wrong to throw an exception in the WebSocketHandshaker, I will look into this as a separate issue :) Would you mind to also create a pull request for 3.x ?

@danbev
Copy link
Copy Markdown
Member Author

danbev commented Sep 9, 2012

Sure no problem, I'll create a pull request for 3.x.

Den 9 sep 2012 11.28, "Norman Maurer" notifications@github.com skrev:

@danbev https://github.com/danbev you handled the future the right way. I
think its wrong to throw an exception in the WebSocketHandshaker, I will
look into this as a separate issue :) Would you mind to also create a pull
request for 3.x ?


Reply to this email directly or view it on
GitHubhttps://github.com//pull/587#issuecomment-8402063.

@normanmaurer
Copy link
Copy Markdown
Member

Thanks a lot!

Sent from my iPhone. Excuse any typos....

Am 09.09.2012 um 12:09 schrieb Daniel Bevenius notifications@github.com:

Sure no problem, I'll create a pull request for 3.x.

Den 9 sep 2012 11.28, "Norman Maurer" notifications@github.com skrev:

@danbev https://github.com/danbev you handled the future the right way. I
think its wrong to throw an exception in the WebSocketHandshaker, I will
look into this as a separate issue :) Would you mind to also create a pull
request for 3.x ?


Reply to this email directly or view it on
GitHubhttps://github.com//pull/587#issuecomment-8402063.

Reply to this email directly or view it on GitHub.

@veebs
Copy link
Copy Markdown
Member

veebs commented Sep 10, 2012

Thanks @danbev :-)

Let me know if you need a hand working on the other enhancements.

Can I also ask for a change of name form WebSocketServerHandshakeHandler to WebSocketServerProtocolHandshakeHandler?

The reason is so that it is clear that the class is to be used with WebSocketServerProtocolHandler.

Another alternative is to just make it an inner class of WebSocketServerProtocolHandler and be done with it??? I'm just worried that users might get confused which handshaker to use.

If you are busy, let me know and I can do it.

Thanks again for your pull request.

normanmaurer added a commit that referenced this pull request Sep 10, 2012
@normanmaurer
Copy link
Copy Markdown
Member

@veebs @danbev I renamed it and made it public as I think it may also be useful for standalone usage.

normanmaurer added a commit that referenced this pull request Sep 10, 2012
normanmaurer added a commit that referenced this pull request Sep 10, 2012
@normanmaurer
Copy link
Copy Markdown
Member

@danbev I also fixed bug which could lead to get the channel closed before the BAD_REQUEST was written to the client

@danbev
Copy link
Copy Markdown
Member Author

danbev commented Sep 10, 2012

@normanmaurer Thanks for your help and I'll make sure to look at the fixes you made so that I don't repeat them.
@veebs Thanks for your feedback and it looks like Norman has taken care of the issues if I'm not mistaken. If not, then please go ahead with these. I'll work on the pull request for 3.x and then add some some documentation to the wiki.
Thanks guys!

@normanmaurer
Copy link
Copy Markdown
Member

Yep I renamed the class

Sent from my iPhone. Excuse any typos....

Am 10.09.2012 um 09:08 schrieb Daniel Bevenius notifications@github.com:

@normanmaurer Thanks for your help and I'll make sure to look at the fixes you made so that I don't repeat them.
@veebs Thanks for your feedback and it looks like Norman has taken care of the issues if I'm not mistaken. If not, then please go ahead with these. I'll work on the pull request for 3.x and then add some some documentation to the wiki.
Thanks guys!


Reply to this email directly or view it on GitHub.

@danbev danbev deleted the websocket-enhancement branch March 18, 2014 13:25
chrisvest pushed a commit to chrisvest/netty that referenced this pull request Apr 2, 2025
netty#587)

…eference

Motivation:

We missed to delete the local reference in some cases due early returns.

Modifications:

Move the delete calls close to the creation calls and so ensure we not
miss to delete local references

Result:

More correct code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants