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

Resolve Compiler Warnings for Windows Build #33

Merged
merged 2 commits into from Oct 31, 2017

Conversation

Projects
None yet
2 participants
@zodiac403
Contributor

zodiac403 commented Oct 27, 2017

Hi @Cylix

I just open a new PR for this topic.

Hi @Cylix

thanks for all the integration work so far! If you take a look at the tacopie build on Windows, you still see 4 compiler warnings. I am trying to get rid of them.

  • 3 warnings are about type conversion. For some reason Win wants the buffer length for sockets as int instead of size_t. Negative buffer length surely are a breat benefit ^^ I added a type conversion macro right at the Winsock2.h API call as I did not see any benefit of carrying this issue wider through the application.
  • 1 warning is about a deprecated inet_ntoa function, which only supports IPv4. As long we stick to IPv4, we should be save to supress this warning (which I did for now). As we still have some work to do for IPv6, I suppressed this warning.

Ah, and I made the PlatformToolset consistent between all build configs.

When all builds pass, this PR would be ready to merge. Any comments are welcome ;)

zodiac403 added some commits Oct 21, 2017

Fix Visual C++ Compiler Warnings:
- Enable 'TreatWarningsAsErrors' in build config.
- For Win, convert socket buffer sizes to INT.
- For Win, suppress IPv6 warning.
@zodiac403

This comment has been minimized.

Contributor

zodiac403 commented Oct 28, 2017

Hi @Cylix
one Travis build fails due to a brew update error. Do you have any idea where this comes from?

$ if [[ "$TRAVIS_OS_NAME" == "osx" ]]; then brew update; fi
error: RPC failed; curl 56 SSLRead() return error -36
@Cylix

This comment has been minimized.

Owner

Cylix commented Oct 28, 2017

@Cylix

This comment has been minimized.

Owner

Cylix commented Oct 31, 2017

Just had a look, that's perfect!
Thanks a lot for taking care of that, I'm merging it :)

@Cylix Cylix merged commit c4e0cf6 into Cylix:master Oct 31, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@zodiac403

This comment has been minimized.

Contributor

zodiac403 commented Oct 31, 2017

Hi @Cylix
thanks for the merge. I love it, when work gets integrated so fast!

I want to use the Win build in one of my projects. For cpp_redis and tacopie this would be a good test, that our build configs work "in the real world" ;). Therefore I needed a tagged version / release of cpp_redis and tacopie.

Is it possible to fork a stable version from the current masters with small effort? As far as I see, there are only 2 commits (tacopie) / 4 commits (cpp_redis) since the last releases. Or do you have a schedule for upcoming features / releases where I can sync with?

That would be awesome!

@Cylix

This comment has been minimized.

Owner

Cylix commented Nov 3, 2017

Hey,

That's not a problem, I'll make you a release :)

@Cylix

This comment has been minimized.

Owner

Cylix commented Nov 3, 2017

Done, let me know if you need anything else!

@zodiac403

This comment has been minimized.

Contributor

zodiac403 commented Nov 12, 2017

Hey @Cylix
sorry for the late response, that release works fine for our dev environment. Thanks for all the quick responses and actions!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment