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

[SCons] Add warnings build options #1007

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Faless
Copy link
Contributor

@Faless Faless commented Jan 19, 2023

Enable scons "warnings=extra" and "werror=yes" in CI.

Fix few spotted warnings.

Draft status:

  • Unexplained duplicated-branches CI gcc error (cannot reproduce locally with gcc 10) when building test
  • The changes in binder_common are slightly different from the upstream godot (include almost the whole file), and would like an opinion.

@Faless Faless added bug This has been identified as a bug enhancement This is an enhancement on the current functionality topic:buildsystem Related to the buildsystem or CI setup labels Jan 19, 2023
@Faless Faless added this to the 4.0 milestone Jan 19, 2023
@Faless Faless requested a review from akien-mga January 19, 2023 15:51
@Faless
Copy link
Contributor Author

Faless commented Jan 19, 2023

scons part of #999 , thanks a lot to @asmaloney for doing the initial work on fixing warnings.

@asmaloney
Copy link
Contributor

Excellent! I'm CI-testing my cmake changes right now too , so I will wait for this to be merged.

@asmaloney
Copy link
Contributor

This is odd because I'm not getting any warnings on the files you changed... I've checked my option conversion a couple of times and it looks OK (granted my ☕ level is low). I wonder if I'm missing something...

@Faless
Copy link
Contributor Author

Faless commented Jan 19, 2023

@asmaloney the error with the scons CI seems to come from building the test library, and the cmake counterpart seems to always use the isystem strategy after #1002 .

I wonder if we should do the same with the scons builds.

@aaronfranke aaronfranke modified the milestones: 4.0, 4.x Jul 8, 2023
@Faless Faless force-pushed the ext/warnings branch 2 times, most recently from 6acd2ad to 36c43a5 Compare December 1, 2023 10:23
@Faless Faless changed the title [SCons] Add warnings build options. [SCons] Add warnings build options Dec 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This has been identified as a bug enhancement This is an enhancement on the current functionality topic:buildsystem Related to the buildsystem or CI setup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants