Cmake config to treat compile warnings as errors

[codecurve]

Should we treat compile warnings as errors? If so, the Cmake config needs to be updated accordingly.

[nickerso]

yes.

[agarny]

I clearly would... :)

[codecurve]

I'm happy either way, so I'd say we should do it. I'll open a pull request soon.

[hsorby]

I wouldn't treat warnings as errors, I would just have it that no code would be accepted if it created warnings on the buildbot test machines. The reason for this is that other users will have newer compilers than what we are testing with and invariably they will get a warning that will then stop the code from building. I think that this is very undesirable and we should therefore not have warnings as errors.

[agarny]

If that was ever to happen (i.e. someone using another compiler that generates a warning), then it might be an opportunity to 'fix'/improve the codebase. So, personally, I would think of it as being a good thing.

Alternatively, maybe we could have a flag that allows or not warnings to be treated as errors? For BuildBot, that flag would be on (i.e. warnings = errors). Just a thought.

[codecurve]

We decided to tack the "warnings are errors" setting on as part of #49 before @hsorby 's comment, and rather than as a separate pull request. I could revert that, and wait for the decision on this to be resolved, so that this decision does not hold up that pull request. What do others think?

[hsorby]

It will happen that can be guaranteed. I definitely don't see it as a good thing that someone has to 'fix' the code before they can build it. Especially when the 'fix' is for a warning that is being treated as an error.

I would add an option to allow setting warnings as errors, especially for the buildbot machines, even with an option I would set the default to off.

[agarny]

I am happy with an option to be set to off by default, and to on for buildbot machines.

[codecurve]

Cmake side done (see c5aef13).
@hsorby still to do setting on BuildBot.

[nickerso]

If we are going for an option, my vote would be to default it on.

[codecurve]

I prefer the option of having "treat-warnings-as-errors" off by default, but enabled on the buildbots (i.e. same as @agarny and @hsorby ). I'll have it enabled on my local systems when I develop, rather than leaving it to the buildbot to catch warnings.