-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
Reduce warnings during compilation with all (usual) warnings switched on #2746
Comments
Related: #2669 (discusses which warnings to enable). I think treating warnings as errors should be an opt-in. Disabled by default, but enabled in CI pipelines. As usual, I would look at OpenCV scripts to check how they handled this. |
I agree with @taketwo on this one. |
Aiming at reduction in windows warnings, I counted them:
And other warnings from tests only (lower priority). # edit before running
curl https://dev.azure.com/PointCloudLibrary/0fc52e87-00b9-420e-acbc-c842c4f2d9cc/_apis/build/builds/15904/logs/80 | \
grep warning | grep [-v] test | grep <warning> | wc -l Some of these are good warnings for beginners to tackle (like 4477, 4244, 4146, etc.). Others not so much. I don't know if |
To treat as warnings as errors you have to set flag See also MSVC documentation for these flags and list of all warnings and their level. |
I was wondering if we set it on the CI because IIRC, windows flags are set in CMake. If no, then the warnings will only increase |
As far as I know you cannot enable/disable single warnings on MSVC. As we have already enough warnings, I think current level is okay. So maybe we should start to enable |
I found some references before that suggest otherwise.
|
Ahh good to know (should have read my link completly myself 😆) |
To increase code quality of new PR, we should treat warnings as errors (as already mentioned here #2733).
In case we have not solved all compiler warnings, before introducing this change, we have to whitelist this warning types (global or per target).
Because with new compilers sometimes new compiler warnings will be introduced, we should care about this:
-Werror
explicitly list all warning types which should treat as error (this list could be really long), so new warnings will be not treated as errorBefore we can introduce this, we need to resolve #2732 & #2745, so 3rd-party code cannot raise warnings.
The text was updated successfully, but these errors were encountered: