search

NCAR / VAPOR

VAPOR is the Visualization and Analysis Platform for Ocean, Atmosphere, and Solar Researchers
https://www.vapor.ucar.edu/
BSD 3-Clause "New" or "Revised" License
178 stars 49 forks source link

Remove compilation warnings for Master #704

Open shaomeng opened 6 years ago

shaomeng commented 6 years ago

We currently have thousands of compilation warnings in the Master branch. For a high quality product, we should NOT have them. We could possibly require any code merged into master don't have any warnings.

clyne commented 6 years ago

Agreed. We skipped over this (it’s on the schedule with your name assigned to it, Sam :-). We’ve migrated to C++11 and need to agree upon the new compiler (including version) and flags to use to generate the warnings.

Re requiring code committed to master to not have compiler warnings: on which platforms? They all have different degrees of pickiness wrt complier warnings. So not sure that this is practical.

jc

On Apr 24, 2018, at 10:36 AM, Samuel Li notifications@github.com wrote:

We currently have thousands of compilation warnings in the Master branch. For a high quality product, we should NOT have them. We could possibly require any code merged into master don't have any warnings.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

John Clyne National Center for Atmospheric Research 303.497.1236 (w), 303.809.1922 (c) clyne@ucar.edu

shaomeng commented 6 years ago

Regarding practical question, I think it is. A piece of evidence is that the VTKM project imposes such requirement, and it seems to be working well.

Regarding to which platform, again, VTKM tests it on ALL platforms with many versions of compilers, so I'd like VAPOR to be warning free on all 3 platforms as well.

Lastly, even though all compilers have different characteristics, if your code is warning free on one, it's not likely to have many warnings on another.

clyne commented 6 years ago

We should address:

hidden overloaded virtual functions (Mac only), which are currently disabled with -Wno-overloaded-virtual