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

Sockets: docs for IPAddress, Addrinfo + some helpers #3781

Conversation

ysbaddaden
Copy link
Contributor

@ysbaddaden ysbaddaden commented Dec 26, 2016

  • Adds documentation to Socket::IPAddress and Socket::Addrinfo, to differentiate how they are different: one holds an IP address when the other will resolve against a DNS server or /etc/hosts.
  • Adds overloads to Socket::Addrinfo that return an Array(Socket::Addrinfo) instead of yielding results.
  • Adds argless constructors to TCPSocket and TCPServer that don't connect nor bind.

@ysbaddaden
Copy link
Contributor Author

I'm making some additions / corrections.

@ysbaddaden ysbaddaden force-pushed the std-sockets-docs-and-resolve-domains-as-array branch from fb12839 to b7575aa Compare December 26, 2016 13:29
@ysbaddaden
Copy link
Contributor Author

  • Fix Socket::Addrinfo#resolve to return more than one result.
  • Better documentation for how Socket::Addrinfo#resolve(&block) expects the block to behave.
  • Improved Socket::Addrinfo#resolve(&block) to only raise if the returned value is an Exception, otherwise returns the value.
  • Fixed Socket::Addrinfo#ip_address that failed to compile.
  • Improved specs to catch the above issues.

@ysbaddaden
Copy link
Contributor Author

I think I'm done. What do you think?

@asterite
Copy link
Member

@ysbaddaden Looks good to me, though it's true what @miketheman mentions that IPAddress currently has a port, which feels odd (but it had it before, so we can probably improve/fix this after merging this PR). So merge if you want :-)

@ysbaddaden
Copy link
Contributor Author

Well, it's actually a Socket::Address aka sockaddr in POSIX, not just an IPAddress.

It got an IP prefix because it's different from an UNIXAddress and nicer than the in suffix of sockaddr_in.

@asterite
Copy link
Member

Let's merge this :-)

@ysbaddaden Thank you!

@asterite asterite merged commit 48af685 into crystal-lang:master Dec 27, 2016
@asterite asterite added this to the 0.20.4 milestone Dec 27, 2016
@ysbaddaden ysbaddaden deleted the std-sockets-docs-and-resolve-domains-as-array branch June 27, 2018 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants