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

Fix more compiler warnings #204

Merged
merged 4 commits into from
Feb 1, 2017
Merged

Fix more compiler warnings #204

merged 4 commits into from
Feb 1, 2017

Conversation

jamiesnape
Copy link
Contributor

PTAL @jslee02.

Copy link
Member

@jslee02 jslee02 left a comment

Choose a reason for hiding this comment

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

Looks good to me! I have one suggestion removing -Wno-unused-parameter as commented below.

@@ -1,14 +1,14 @@
# GCC
if(CMAKE_COMPILER_IS_GNUCXX)
add_definitions(-std=c++11 -W -Wall -g -Wextra -Wpedantic -Wno-missing-field-initializers -Wno-unused-parameter)
add_definitions(-std=c++11 -W -Wall -g -Wextra -Wpedantic -Wno-unused-parameter)
Copy link
Member

Choose a reason for hiding this comment

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

I believe we could remove -Wno-unused-parameter as well. One case could be a problem I can think of is using variables that are only used in assertions. For that case, we could use a macro to suppress the warning as we did in DART.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Almost, I have another PR for that. Touches something like 36 files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See #205.

@jslee02 jslee02 added this to the FCL 0.6.0 milestone Feb 1, 2017
@jslee02 jslee02 merged commit 6cfdd83 into flexible-collision-library:master Feb 1, 2017
@jamiesnape jamiesnape deleted the fix-more-warnings branch February 2, 2017 14:00
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.

2 participants