6 ms·
I'm sticking by the principle that warnings should be warnings and errors should be errors, and that you shouldn't "promote" warnings to be errors. I can't reme
by daemin 5y ago
I'm sticking by the principle that warnings should be warnings and errors should be errors, and that you shouldn't "promote" warnings to be errors. I can't remember who said it but I think it was in a conference video.
People often claim that without promoting warnings to be errors and thus stopping compilation that you'll just get a lot of warnings. However in reality most of these warnings just get disabled in the code, and often in a way that disables them for the entire project, meaning that you're missing out on good diagnostic warnings.
The proper way to fix warnings is to keep them as warnings and have it on the team to make sure they don't get out of hand. By making sure when you implement a new feature or fix a bug that you don't introduce new warnings, and also have people investigate warnings and fix them in the correct and sustainable way. In some cases you would disable that warnings but in general they should be fixed by fixing the code. That way you get the benefit of having good diagnostics along with the flexibility of being able to investigate and fix it at a later date.
One final note is that having warnings show up during the compile is a good indicator of how much technical debt there is in the project. If you just end up disabling warnings you're really hiding how bad the code state is.
- adrianN 5y agoIf you disable warnings such that they're disabled for the whole project you're just doing it wrong imho. Warnings need to be fixed by making the compiler happy and only (locally!) disabling them if there is a bug in the compiler.
- daemin 5y agoYes you should only disable them locally, in as small a region as possible. Though in a lot of cases in my experience this isn't so. They're generally bulk disabled before including external third party headers, they're disabled in low level projects, they're disabled in headers so that they can be submitted. The point is that people often disable them in ways which they shouldn't just so they can compile their code and get on with fixing their bug or implementing their feature. Mainly because warnings get treated like errors and prevent the build from succeeding. With warnings as warnings we could take a more caretaker approach to dealing with them and we shouldn't have these situations created.
- lupire 5y agoWhy do you think that people who are so undisciplined that they disabled warnings, are so disciplined that they will fix warnings? Not disabling a warning is far easier than fixing it. A warning is only useful if it's someone else's place to fix and it's warning you that you might suffer or you they might suffer from misusing you code. Warnings are most useful at runtime where it's impossible to stop and fix.
- adrianN 5y agoI don't know. If your process is not up to the task of preventing people from bulk-disabling warnings, I wouldn't trust the process to adequately inspect warnings that don't prevent shipping.
- daemin 5y agoWell it will require a behaviour change either way. I think that keeping warnings as warnings will at least make it visible how much debt there is to fix in the project rather than hiding it behind warning disabling pragmas.
- bjourne 5y agoI think your comment illustrates well why treating "warnings as warnings" is counterproductive. People who treat warnings as warnings haven't taken a caretaker approach and so they have accumulated and are now so numerous that bulk disabling them is the only practical option!
- daemin 5y agoI have worked at (and are currently working at) places that treat warnings as errors, and if the warning is not trivial to fix it just ends up disabled. People do notice it during code reviews but it usually goes through as someone "will fix it later". So out of sight and out of mind.
- outsomnia 5y ago> The proper way to fix warnings is to keep them as warnings and have it on the team to make sure they don't get out of hand. "Proper"... no, warnings just scroll by and are forgotten. Error stop the build and make you keep the number of them at zero. If you allow any warnings, they will increase according to the tolerance of the most lax person on your team. And you will never spend the time on the technical debt to clean them. You are accepting an increasing number of things wrong with your codebase that you will never investigate nor regain control of.
- devwastaken 5y agoEither you have warnings, or you have no warnings. The problem with no warnings is not that the warnings were "fixed", but that they get disabled. That is far more problematic.
- krzyk 5y agoDisabled or just shown, doesn't matter they will proliferate unless turned into errors.
- daemin 5y agoIf they're disabled then they will proliferate even if they are turned into errors. Better to be visible and culled promptly than disabled in code and hidden away.
- krzyk 5y agoWhy would you disable them? You fix it and done. If you start your project with 0 warnings and enforce it in CI it is responsibility of developer who adds the code to make it warnings free and not disable the warning (that needs to be checked in code review).
- krzyk 5y agoWhy would you disable them? You fix it and done.
- bhaak 5y agoYou can promote only specific warnings, for example -Werror=format. This way you can still use the compiler to stop you reliably from committing unwanted code constructs.
- krzyk 5y ago> People often claim that without promoting warnings to be errors and thus stopping compilation that you'll just get a lot of warnings. This, do you know people that look at warnings that don't break the code/compilation? Those are usually ignored, that's why one can promote them to errors.