Use -Werror in CI #237
Use -Werror in CI #237
Conversation
The PG15 support that was introduced in #180 introduced a bunch of warnings when compiling. To avoid getting numb to warnings, this adds the -Werror to our CI builds, so we catch warnings before merging to main. This also starts to ignore the register warning which was introduced by PG15 support.
| @@ -113,7 +113,7 @@ jobs: | |||
| id: build | |||
| run: | | |||
| pushd duckdb | |||
| make -j8 install | |||
| make -j8 install CFLAGS=-Werror CXXFLAGS=-Werror | |||
There was a problem hiding this comment.
Why not using it in the Makefile as well? Too annoying in dev cycles?
In this case, maybe we want a dedicated make target?
There was a problem hiding this comment.
Why not using it in the Makefile as well? Too annoying in dev cycles?
Yeah, many warnings don't really matter during development (e.g. unused variable).
Also, Having -Werror on by default is bound to break builds when users compile using newer compilers than our CI does. e.g. people that create packages for distros hate it when people set -Werror.
If there's specific warnings that we think we should always treat as an error, we can do that in the makefile by default though. e.g. one I usually set to error is -Werror=return-type. But let's do that in a separate PR.
In this case, maybe we want a dedicated make target?
That would kinda explode the make targets I think, because we'd need an additional make target for all/install/installcheck and maybe more. So I think the solution in this PR is probably easiest.
There was a problem hiding this comment.
Fair enough - if we need this we can always add it later.
The PG15 support that was introduced in #180 introduced a bunch of warnings when compiling. To avoid getting numb to warnings, this adds the -Werror to our CI builds, so we catch warnings before merging to main.
This also starts to ignore the register warning which was introduced by PG15 support.