7 ms·
Agreed. New code shouldn't be committed unless it compiles without error. The build server should then compile with -Werror, with all relevant shame being hea
by MereInterest 11y ago
Agreed. New code shouldn't be committed unless it compiles without error. The build server should then compile with -Werror, with all relevant shame being healed on the developer who breaks the build by letting a warning through.
- ryandrake 11y agoSadly, you folks seem to be the minority out there. I can't even remember how many times I've had conversations along the lines of: Me: Hey, that code generates 3 compiler warnings, and you shouldn't be casting that pointer here. Them: Oh, ok, whatever, change it if you want. Me: It's not about what I want.. the code isn't correct. It'll break on some 64-bit platforms, for instance. Them: Well, it works on our target platform, and that's the one in the spec. If the spec changes, we fix it as a change request. Me: But it just happens to work! Even a different compiler could result in different behavior. Can't we just put in a little effort up front to avoid trouble later? Them: Go for it if it bothers you so much. We have real bugs to fix. Nobody cares about hypothetical problems on systems we don't support and compilers we don't use. Me: [This is going to drive me to alcoholism]
- blazespin 11y agoThe biggest issue with lettings warnings go is that they pile up and you start missing the important ones that point to real bugs. I have seen that happen many many times.
- MereInterest 11y agoI think that I managed to convince a group member that we should have "-Wall -Wextra -pedantic" enabled all the time. He had disabled the warnings because they annoyed him. Yesterday, we spent a large quantity of time hunting down a bug that could have been avoided entirely had "-Wreturn-type" been enabled.
- to3m 11y agoAnd at the same time, while the development team is having fun with the blame game, somebody is waiting for a build that never comes! At least do it the right way round: developer machines build with -Werror, and build PC should build without. (Selective "promotion" to errors is a good idea on top of that.) That way, it's not the end of the world if people check something in that doesn't build cleanly, because they disabled -Werror on their local copy in order to get some debugging done without having to play whack-a-mole as they did so. (Obviously, they're not supposed to do that. But ideally they wouldn't be doing any debugging in the first place, either. These things happen, and processes should be designed around that.) Not all warnings are equal, and -Werror doesn't give you any discretion. You probably want to be kept abreast of unused functions, unused variables, unused parameters, int-to-bool casts, truncation of double literals, and so on - but they're poor justifications for stopping the build. You don't need to hold the build to ransom to get it to compile without warnings. All you need to do is take the warnings out.
- SamReidHughes 11y agoI've never had big blocking problems like that with -Werror on all the time. You get one? Fix it and move on. When you have a large enough organization or enough different compiler versions at play, disable the specific warnings that seem to get you in trouble between compiler versions.
- MereInterest 11y agoNot all warnings are equal, but all warnings get printed to screen. If there are heaps and loads of ignoreable warnings, then I can't tell whether somewhere in that mess is a warning that I should be paying more attention to. Better to have a policy that no warnings should occur.
- to3m 11y agoI'm not saying they shouldn't be fixed. They should. They just don't need to be errors.
- dllthomas 11y agoThe biggest place this breaks down in my experience (which is not to say it's not a win overall!) is when a compiler upgrade introduces a new warning. There's several ways it can play out, some of which aren't a bad thing overall, but most of which involve some pain.