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

Unify handling of single-file -Werror in all modules #3910

Merged
merged 2 commits into from
Jan 17, 2023

Conversation

dillona
Copy link
Contributor

@dillona dillona commented Jan 15, 2023

This implements unified handling and flags like I was thinking here

nilsnolde
nilsnolde previously approved these changes Jan 15, 2023
Copy link
Member

@nilsnolde nilsnolde left a comment

Choose a reason for hiding this comment

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

Thanks for the cleanup @dillona ! Esp for finding and protecting more source files!

COMPILE_FLAGS
"-Werror -Wall -Wextra -Wno-unused-parameter -Wno-missing-field-initializers")
endif()
list(APPEND MODULE_SOURCES ${MODULE_SOURCES_WITH_WARNINGS})
Copy link
Member

Choose a reason for hiding this comment

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

nit: I guess appending to MODULE_SOURCES can go into the if?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do not think so, as we need to ensure all files are compiled, regardless of the Werror configuration

Copy link
Member

Choose a reason for hiding this comment

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

Ah right, sorry, wasn't looking into the code that followed this 😅 Then all good!

kevinkreiser
kevinkreiser previously approved these changes Jan 16, 2023
@kevinkreiser kevinkreiser dismissed stale reviews from nilsnolde and themself via 4725df5 January 17, 2023 14:35
@kevinkreiser kevinkreiser merged commit 6879ce7 into valhalla:master Jan 17, 2023
@dillona dillona deleted the unify-werror branch January 25, 2023 04:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants