Skip to content
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

Closed
wants to merge 4 commits into from
Closed

Conversation

etehtsea
Copy link
Contributor

Also refactor (un)pack_sockaddr code.

@@ -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) {
Copy link
Member

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.

@enebo
Copy link
Member

enebo commented Aug 12, 2016

@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.

@etehtsea
Copy link
Contributor Author

@enebo done

@headius
Copy link
Member

headius commented Aug 17, 2016

@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.

@etehtsea
Copy link
Contributor Author

@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.
Otherwise it can become never-ending story.

P.S. Interesting question about resolving conflicts: should I leave your commits intact and fix conflicts in another commit or fix them inside yours?

@headius
Copy link
Member

headius commented Aug 18, 2016

@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.

@etehtsea
Copy link
Contributor Author

Superseded by #4093

@etehtsea etehtsea closed this Aug 19, 2016
@enebo enebo modified the milestone: Non-Release Aug 26, 2016
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.

None yet

3 participants