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

Added Windows support with win_inet_pton #38

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

bgianpetro
Copy link

I found this in the 'Issues' and decided to provide a fix. I know it's not preferable to have the external dependency on 'win_inet_pton', but that was the only way I could get it working. If this isn't ideal, I can just a blurb to the README for Windows users to note that an install and conditional include of 'win_inet_pton" is required.

@coveralls
Copy link

coveralls commented Mar 12, 2018

Coverage Status

Coverage decreased (-0.2%) to 90.175% when pulling 873e929 on bgianpetro:master into 52d70b0 on mjschultz:master.

@mjschultz
Copy link
Owner

No, I think this is fine. But I would like a blurb in the README stating that Windows does depend on this external library. I think something like (in the installation section):

Windows
^^^^^^^
Windows support depends on a third-party library--`win-inet-pton`--to provide `inet_pton` and `inet_ntop` wrappers for the socket library. It is automatically included for installations on the Windows platform.

@bgianpetro
Copy link
Author

Ok, I added that into the README verbatim (it looked good to me!)

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