Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove -Werror from autotools darwin build #800

Merged
merged 1 commit into from
Mar 31, 2023

Conversation

dechamps
Copy link
Contributor

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.

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.
@dechamps
Copy link
Contributor Author

@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
Copy link
Collaborator

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

r-burns #636 (comment) 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 #636 (comment) wrote:

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

And later in the thread #636 (comment) :

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 #636 (comment) wrote:

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

And later in the thread #636 (comment) :

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

@RossBencina
Copy link
Collaborator

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:
#803

We should discuss the approach on that PR.

@RossBencina RossBencina self-requested a review March 28, 2023 00:26
Copy link
Collaborator

@RossBencina RossBencina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, but let's fix CI too. See #803 for that.

@RossBencina RossBencina added the final merge review Maintainers flag as ready to make a final merge decision label Mar 28, 2023
@philburk philburk merged commit bc3ad02 into PortAudio:master Mar 31, 2023
@dechamps dechamps deleted the configurewerror branch May 25, 2024 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build-autoconf Autoconf build system final merge review Maintainers flag as ready to make a final merge decision
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants