4 ms·

> 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 scr

> 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 ago
Either 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 ago
  Disabled or just shown, doesn't matter they will proliferate unless turned into errors.
  - daemin 5y ago
    If 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 ago
      Why 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 ago
      Why would you disable them? You fix it and done.
nullc 5y ago
> no, warnings just scroll by and are forgotten Sounds like a bad team/management, not anything to do with werror. Plenty of groups have no problems maintaining warning free code on a target toolchain without using Werror. Disrupting what people are working one sometimes encourages "just make it go" fixes that introduce bugs to silence the warning. If the simplest change that silenced the warning was always the right one the compiler could just do it for you and not warn. :)
daemin 5y ago
I never said it was easy, it does require the team to have some discipline about it, but it doesn't sweep the issue that the warning was about under the rug, hidden from view. If you have a lot of warnings then you have a lot of issues with the code. Would you rather have them visible or disabled?
- outsomnia 5y ago
  They are not 'visible' if you just let them scroll by. If there are individual warnings that have no meaning, turn them off. Everything else, is help from the compiler, which is your only friend. Ignoring even one of those should not be tolerated at all. They must be analyzed and the code improved to remove them. And you enforce your team following that policy with -Werror.
  - daemin 5y ago
    Yeah, but push comes to shove during shipping, people leave, and the pragmas remain in the code in various places. At least if they are scrolling by then people can get fed up with that and take some initiative to fix those warnings. Or the new people joining your studio can come in and try to fix them. It's much more difficult to get people to fix pragmas scattered throughout the code. Which ones were legit, which ones were put in at the last moment out of frustration, which ones were put in temporarily years ago? I think a softer approach works better since people won't be so quick to work around the compiler by disabling the warnings.
saagarjha 5y ago
> 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. This is making a lot of assumptions about team dynamics, and fails to account for the person who notices build warnings creep and aggressively sends in patches to fix them.

devwastaken 5y ago

Either 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 ago
Disabled or just shown, doesn't matter they will proliferate unless turned into errors.
- daemin 5y ago
  If 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 ago
    Why 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 ago
    Why would you disable them? You fix it and done.

nullc 5y ago

> no, warnings just scroll by and are forgotten Sounds like a bad team/management, not anything to do with werror. Plenty of groups have no problems maintaining warning free code on a target toolchain without using Werror. Disrupting what people are working one sometimes encourages "just make it go" fixes that introduce bugs to silence the warning. If the simplest change that silenced the warning was always the right one the compiler could just do it for you and not warn. :)

daemin 5y ago

I never said it was easy, it does require the team to have some discipline about it, but it doesn't sweep the issue that the warning was about under the rug, hidden from view. If you have a lot of warnings then you have a lot of issues with the code. Would you rather have them visible or disabled?

outsomnia 5y ago
They are not 'visible' if you just let them scroll by. If there are individual warnings that have no meaning, turn them off. Everything else, is help from the compiler, which is your only friend. Ignoring even one of those should not be tolerated at all. They must be analyzed and the code improved to remove them. And you enforce your team following that policy with -Werror.
- daemin 5y ago
  Yeah, but push comes to shove during shipping, people leave, and the pragmas remain in the code in various places. At least if they are scrolling by then people can get fed up with that and take some initiative to fix those warnings. Or the new people joining your studio can come in and try to fix them. It's much more difficult to get people to fix pragmas scattered throughout the code. Which ones were legit, which ones were put in at the last moment out of frustration, which ones were put in temporarily years ago? I think a softer approach works better since people won't be so quick to work around the compiler by disabling the warnings.

saagarjha 5y ago

> 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. This is making a lot of assumptions about team dynamics, and fails to account for the person who notices build warnings creep and aggressively sends in patches to fix them.