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

build_system: remove unused-parameter flag for AppleClang #354

Merged
merged 1 commit into from
Apr 9, 2018

Conversation

JonasVautherin
Copy link
Collaborator

Sources generated by protobuf do not compile otherwise. I tried to set the flag only for the generated files with set_source_files_properties, but didn't manage to make it work this way.

Sources generated by protobuf do not compile otherwise.
@@ -5,6 +5,8 @@ set(CMAKE_CXX_STANDARD 11)

list(APPEND CMAKE_FIND_ROOT_PATH ${CMAKE_BINARY_DIR}/third_party)

cmake_policy(SET CMP0025 NEW)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add a comment why this was needed.

Copy link
Collaborator Author

@JonasVautherin JonasVautherin Apr 9, 2018

Choose a reason for hiding this comment

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

I can do it if you insist, but I tend to disagree on that :-). "CMP0025" is so unique that you cannot miss the CMake documentation when Googling it, and its first line says:

Compiler id for Apple Clang is now AppleClang.

I generally don't like adding comments, but I agree it is necessary sometimes when we have to do weird tricks. This is not a trick at all. To me it would be the same as as putting a comment above std::shared_ptr<...> to explain what a shared pointer is. It is general knowledge, and I believe my source code is not the place for general documentation :D. Comments tend to get outdated and clutter the code.

Again, if you insist, I can add a comment there :D.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, agreed.

@@ -5,6 +5,8 @@ set(CMAKE_CXX_STANDARD 11)

list(APPEND CMAKE_FIND_ROOT_PATH ${CMAKE_BINARY_DIR}/third_party)

cmake_policy(SET CMP0025 NEW)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, agreed.

@julianoes julianoes merged commit f819bc1 into develop Apr 9, 2018
@julianoes julianoes deleted the fix-apple-build branch April 9, 2018 13:37
rt-2pm2 pushed a commit to rt-2pm2/DronecodeSDK that referenced this pull request Nov 27, 2018
build_system: remove unused-parameter flag for AppleClang
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