-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Upgrade to Netty 4 #2
Comments
Interesting idea. Without having looked at Netty 4 much, supporting both version 3 and 4 would require two separate implementations, right? Our push server at Battlelog, Beaconpush, has been the main driver in the API design for sockjs-netty. In Beaconpush, we haven't yet migrated to Netty 4 as it was just recently released. But if more people are willing to help port sockjs-netty over to Netty 4 I am of course grateful. That is the version to support moving forward for sure. |
There's definitely no reasonable way to support both APIs in one version of the library. It would probably make the most sense keep Netty 3 support in a branch so it can continue to be fixed/updated/etc. The Netty 4 APIs are significantly different in some aspects, so the conversion cannot be entirely rote, but it's not too difficult either -- it's mostly a matter of understanding which events in the new ChannelHandler interface map to the old ones, and where their semantics differ. There's a pretty good writeup on the whole process here: http://netty.io/wiki/new-and-noteworthy.html I'll keep plugging away at it in my spare time, and let you know when I have something worth reviewing. And if you or anyone else decides to take this on before I finish, please let me know so we don't duplicate too much work. Thanks! |
Cool, keep going :) |
I'm entering this primarily to make sure that I'm not duplicating work. I need sockjs support for Netty 4 in my own work, and this appears to be the most complete sockjs library I've found for Netty (there are a number of reasons we can't use Vert.x).
I've begun the upgrade process in a fork, and it doesn't look like it's going to be too much work, but I obviously don't want to do duplicate work if you're already working on it.
The text was updated successfully, but these errors were encountered: