Re: Do not use -Werror by default in your modules without joining #testable

[philip chimento gmail com]

On Sun, Apr 2, 2017, 10:21 Sébastien Wilmet <swilmet gnome org> wrote:

On Sun, Apr 02, 2017 at 04:52:49PM +0100, Emmanuele Bassi wrote:
> You seem to misunderstand what a continuous delivery/continuous
> integration pipeline is for.
<snip>

I think I understand what a CI server is for, I simply disagree with
you.

Let's compare two scenarios:

1) This warning: comparison between signed and unsigned integer.
2) A real build error due to a change in an underlying library.

For the sake of argument, 1) can be replaced by any warning that becomes
an error if -Werror is enabled. I.e. not a "real" build failure, by
default it's just a warning.

In most cases, if 1) appears, the problem is located in the code of the
module itself, it's not caused by a dependency. So the developer
directly sees it when building the module in jhbuild, even if the deps
are not up-to-date.

For 2), it's better that it is detected by a CI server so that we know
the problem as soon as possible.

A lot of GNOME modules have compilation warnings, and I don't consider
them critically important. In fact, -Werror is disabled in tarballs
(what we actually ship to distros). Of course it's better to fix them,
and in the modules that I maintain they are all fixed except deprecation
warnings. I won't push a commit on the master branch if it adds a
warning, because I directly see the warning when building the code
locally.

On the other hand it's nice that the CI server detects 2) because it's
not practical to rebuild the dependencies in jhbuild all the time.

I think you might still be misunderstanding what Emmanuele is asking.

If you don't believe that the CI server should detect case (1), or you prefer to have your own watchfulness as the last line of defense, then simply don't enable Werror by default on your module. Use a configuration locally that allows you to catch the warnings you want before you push, and there's no need to bother with the CI server.

If you _do_ believe that the CI server should detect case (1), then make Werror the default on your module, and watch the CI server notifications in case some warning slips past you when you push, so that other people are not inconvenienced.

My understanding was the objection was against using Werror as the default while _not_ paying attention to the CI notifications. Please correct me if I've misunderstood this.

Regards,

Philip C