-
-
Notifications
You must be signed in to change notification settings - Fork 925
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
Add Socket.unpack_sockaddr #4066
Conversation
f4f3278
to
3f55a17
Compare
@@ -130,58 +130,6 @@ public static IRubyObject getservbyname(ThreadContext context, IRubyObject[] arg | |||
return runtime.newFixnum(port); | |||
} | |||
|
|||
public static IRubyObject pack_sockaddr_in(ThreadContext context, IRubyObject port, IRubyObject host) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@etehtsea Since these are public static methods we should deprecate and call to the new location since there is a small possibility some Java Native Extension could be calling them.
@etehtsea just make the older versions appear again and I think this is good (by appear I mean add the signatures back, add @deprecated anno, and have it's body call to the new location. |
3f55a17
to
5146889
Compare
@enebo done |
5146889
to
a97733d
Compare
@etehtsea Have you looked at my ruby-2.3+socket branch? I'm VERY eager to get help with it, but it has (re)implemented a number of Socket features to try to conform to @yorickpeterse's specs for rubysl/rubysl-socket as well as MRI's tests. Lots of work on sockaddr packing and unpacking, Addrinfo, and more. If you can help me with some of that maybe we could have it ready by end of year to land with JRuby 9.2 + Ruby 2.4 support. Or sooner, if it really looks stable. |
@headius I have an idea to merge it by small isolated portions. What do you think about these:
After this I can start to resolve conflicts on other that don't apply now. P.S. Interesting question about resolving conflicts: should I leave your commits intact and fix conflicts in another commit or fix them inside yours? |
@etehsea This is great work! Yes, I think we should start merging your changes in, but probably against a branch so we don't destabilize 9.1.3.0 right before we release it. I propose we pull a branch off master and merge the PRs there for now, and after the 9.1.3.0 release we'll evaluate stability and get that branch back on master. Regarding conflicts: I'd say it's up to you. If you feel like my original commit/code is worth fixing, go that route. But I'm not married to anything on that branch, and if you can get more of it in you should feel free to do it however works best. |
Remove (un)pack_sockaddr specs from excludes
a97733d
to
f87b04d
Compare
Superseded by #4093 |
Also refactor (un)pack_sockaddr code.