PortAudio / portaudio

PortAudio is a cross-platform, open-source C language library for real-time audio input and output.
Other
1.43k stars 296 forks source link

Remove -Werror from autotools darwin build #800

Closed dechamps closed 1 year ago

dechamps commented 1 year ago

Warnings should not be turned into errors by default as that can cause difficulties for people who just want to build PortAudio, not work on it. Also this is inconsistent with other platforms.

This PR is a spin-off of #780.

dechamps commented 1 year ago

@RossBencina from #780

I thought we'd already made a decision on this, but I don't remember what it was.

You may be referring to the discussion on #636?

RossBencina commented 1 year ago

I checked #636, here are the pertinent comments from there:

r-burns https://github.com/PortAudio/portaudio/pull/636#issuecomment-913055557 wrote:

Werror is good to enable for development, but unless your CI checks every possible compiler out there, warnings can always sneak by. For example, Nixpkgs ran into some new warnings with gcc 10. I think the recommended approach is to keep Werror off by default, and enable it on the CI using CPPFLAGS.

@philburk https://github.com/PortAudio/portaudio/pull/636#issuecomment-913249765 wrote:

I am in favor of merging this after we add -Werror to the CI.

And later in the thread https://github.com/PortAudio/portaudio/pull/636#issuecomment-937346049 :

Ross and I discussed this and we are both OK with removing -Werror from configure and cmake. So we need to have the -Werror in the CI builds.

@RossBencina https://github.com/PortAudio/portaudio/pull/636#issuecomment-915668929 wrote:

I think this is heading in the right direction. It's OK to limit -Werror to CI.

And later in the thread https://github.com/PortAudio/portaudio/pull/636#discussion_r723763544 :

The CI builds should use -Werror for both debug and release builds.

RossBencina commented 1 year ago

So from #636 we already agreed to removing -Werror from our distributed build files, but we want to retain it in CI.

I've created a PR with options for adding it back to autotools CI: https://github.com/PortAudio/portaudio/pull/803

We should discuss the approach on that PR.