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

Enables warnings by default for all compiles of the PlayRho library #485

Merged
merged 1 commit into from
May 3, 2023

Conversation

louis-langholtz
Copy link
Owner

Description - What's this PR do?

I had these enabled in my development environments but had forgotten to add these to CMake. Thanks go to GitHub user "titofra" for pointing out that these were missing from CMake in their PR, #484.

Impacts/Risks of These Changes?

Cleaner builds - in terms of compiler reported warnings/errors - by users including the PlayRho library.

Where should a reviewer start?

The compile logs of the CI workflows.

How should this be tested?

Noticing that no warnings/errors are reported - after possibly addressing any that may now show up.

Related Pull Requests

@louis-langholtz louis-langholtz added Enhancement For suggestions or changes that enhance any part of the project and isn't a bug. CMake Use to associate with the CMake tool. Library For issues that effect the library and aren't specific to any particular application. labels May 2, 2023
@louis-langholtz louis-langholtz added this to the 2.0 Release milestone May 2, 2023
@louis-langholtz louis-langholtz self-assigned this May 2, 2023
@github-actions
Copy link
Contributor

github-actions bot commented May 2, 2023

clang-tidy review says "All clean, LGTM! 👍"

@louis-langholtz louis-langholtz mentioned this pull request May 2, 2023
@coveralls
Copy link

coveralls commented May 2, 2023

Coverage Status

Coverage: 98.767%. Remained the same when pulling c7e616a on add_warns_to_cmake_library_compiles into 7618614 on master.

@louis-langholtz louis-langholtz force-pushed the add_warns_to_cmake_library_compiles branch from ba323ca to 2751939 Compare May 2, 2023 18:31
@github-actions
Copy link
Contributor

github-actions bot commented May 2, 2023

clang-tidy review says "All clean, LGTM! 👍"

@louis-langholtz louis-langholtz force-pushed the add_warns_to_cmake_library_compiles branch from 2751939 to aee5083 Compare May 2, 2023 19:00
@github-actions
Copy link
Contributor

github-actions bot commented May 2, 2023

clang-tidy review says "All clean, LGTM! 👍"

@louis-langholtz louis-langholtz force-pushed the add_warns_to_cmake_library_compiles branch from aee5083 to 8c65bf9 Compare May 2, 2023 19:47
@github-actions
Copy link
Contributor

github-actions bot commented May 2, 2023

clang-tidy review says "All clean, LGTM! 👍"

@louis-langholtz louis-langholtz force-pushed the add_warns_to_cmake_library_compiles branch from 8c65bf9 to a853a2e Compare May 2, 2023 21:09
@github-actions
Copy link
Contributor

github-actions bot commented May 2, 2023

clang-tidy review says "All clean, LGTM! 👍"

I had these enabled in my development environments but had forgotten to
add these to CMake. Thanks go to GitHub user "titofra" for pointing out
that these were missing from CMake in their PR, #484.

Also, this increases the genericity of the GetModuloNext math function
and ensures it returns the intended value.
@louis-langholtz louis-langholtz force-pushed the add_warns_to_cmake_library_compiles branch from a853a2e to c7e616a Compare May 2, 2023 22:13
@github-actions
Copy link
Contributor

github-actions bot commented May 2, 2023

clang-tidy review says "All clean, LGTM! 👍"

@louis-langholtz louis-langholtz merged commit 7c9e94f into master May 3, 2023
@louis-langholtz louis-langholtz deleted the add_warns_to_cmake_library_compiles branch May 3, 2023 00:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CMake Use to associate with the CMake tool. Enhancement For suggestions or changes that enhance any part of the project and isn't a bug. Library For issues that effect the library and aren't specific to any particular application.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants