Skip to content

👩‍🌾 Suggestions on addressing Windows warnings#59

Merged
chapulina merged 4 commits intoahcorde/windows/warningsfrom
chapulina/3/windows_warnings
Nov 11, 2020
Merged

👩‍🌾 Suggestions on addressing Windows warnings#59
chapulina merged 4 commits intoahcorde/windows/warningsfrom
chapulina/3/windows_warnings

Conversation

@chapulina
Copy link
Contributor

@ahcorde opened #58 as I was working on the same thing 😅

I started from the fixes in #38, but also made some additional changes to make sure we're only suppressing what we need to.

I was also targeting Citadel, but I rebased on top of #58 so we can get that one in, and we can backport everything afterwards.

The main updates here are:

  • Remove the blanket warning suppression in favor of more localized suppression. This way, we can be sure we're not unknowingly introducing warnings of the same type.
  • Treated ign-rendering's warnings the same way as in 👩‍🌾 Disable more tests known to fail on macOS and Windows gz-gui#112, with a note to remove the suppression here once they're fixed upstream.
  • Broke apart #include <ignition/rendering.hh> to include only what we use (and reduce warnings in the process)

Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
@chapulina chapulina added 🔮 dome Ignition Dome Windows Windows support labels Nov 4, 2020
@chapulina chapulina requested a review from ahcorde November 4, 2020 20:03
@chapulina chapulina requested a review from iche033 as a code owner November 4, 2020 20:03
@codecov
Copy link

codecov bot commented Nov 4, 2020

Codecov Report

Merging #59 (92341ac) into ahcorde/windows/warnings (6918f4a) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@                    Coverage Diff                    @@
##           ahcorde/windows/warnings      #59   +/-   ##
=========================================================
  Coverage                     76.33%   76.33%           
=========================================================
  Files                            23       23           
  Lines                          2320     2320           
=========================================================
  Hits                           1771     1771           
  Misses                          549      549           
Impacted Files Coverage Δ
src/AirPressureSensor.cc 83.33% <ø> (ø)
src/AltimeterSensor.cc 86.04% <ø> (ø)
src/CameraSensor.cc 73.35% <ø> (ø)
src/DepthCameraSensor.cc 73.33% <ø> (ø)
src/GpuLidarSensor.cc 87.01% <ø> (ø)
src/ImageGaussianNoiseModel.cc 80.00% <ø> (ø)
src/ImuSensor.cc 88.46% <ø> (ø)
src/Lidar.cc 72.97% <ø> (ø)
src/MagnetometerSensor.cc 86.20% <ø> (ø)
src/Noise.cc 73.33% <ø> (ø)
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6918f4a...92341ac. Read the comment docs.

Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
@chapulina chapulina merged commit f87715b into ahcorde/windows/warnings Nov 11, 2020
@chapulina chapulina deleted the chapulina/3/windows_warnings branch November 11, 2020 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🔮 dome Ignition Dome Windows Windows support

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants