-
Notifications
You must be signed in to change notification settings - Fork 101
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
Disable a Windows platform warning. #311
Conversation
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
c8353c5
to
8458bba
Compare
Looks like I missed one in time_win32.c; that should now be fixed. Otherwise, initial CI looks pretty good. I'm going to run a full CI just to make sure that everything still works for all downstreams as well. I expect that to still have one warning on Windows, which needs a separate patch for RViz. CI: |
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.
LGTM!
I'm surprised I couldn't find any issue saying this can happen without using /experimental:preprocessor
, but maybe Windows introduced a bug in MSVC yesterday (?).
The other option is that we're enabling that compiler flag accidentally.
It would be good to double check that.
So we don't have I'm going to go ahead and merge this as-is to reduce our warnings for now. If Windows/MSVC gets fixed, or we figure out that we are accidentally turning on flags, we can always revert this. |
(oh, and all of the failures on Windows are there on the nightlies as well) |
Yeah, I saw that one.
Agreed, I don't think that will affect other places. |
Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
Signed-off-by: Chris Lalancette clalancette@openrobotics.org
This should get rid of most of the warnings we are currently getting from the Windows CI jobs, as seen in https://ci.ros2.org/view/nightly/job/nightly_win_deb/1804/msbuild . See the diff for a better explanation of why we have to do this.