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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix double declaration of protobuf and construction of the binary #66

Merged
merged 1 commit into from
Apr 11, 2024

Conversation

sodomon2
Copy link
Contributor

No description provided.

the link of the binary was reduced to a single line as it is more practical to reduce the number of lines and also to avoid the link_target so many times.
@@ -22,7 +22,7 @@ elseif(${CMAKE_SYSTEM_NAME} STREQUAL "Linux")
set(PLATFORM_NAME "linux")
endif()

message(STATUS "PLATFORMA_NAME ${PLATFORM_NAME}")
message(STATUS "PLATFORM_NAME ${PLATFORM_NAME}")
Copy link
Owner

Choose a reason for hiding this comment

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

fix a typo in CMakeLists.txt

find_package(cxxopts CONFIG REQUIRED)
target_link_libraries(server-cmd PRIVATE cxxopts::cxxopts)
target_link_libraries(server-cmd PRIVATE asio::asio spdlog::spdlog protobuf::libprotobuf cxxopts::cxxopts)
Copy link
Owner

Choose a reason for hiding this comment

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

target_link_libraries won't do link many times. It's a kind of declaration. After CMake configure the project, it will generate only one link command. One line or Multiline target_link_libraries have no difference.

find_package(spdlog CONFIG REQUIRED)
target_link_libraries(server-cmd PRIVATE spdlog::spdlog)

find_package(Protobuf CONFIG REQUIRED)
Copy link
Owner

Choose a reason for hiding this comment

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

Repeat Find Protobuf

@mkckr0 mkckr0 merged commit 60286ea into mkckr0:main Apr 11, 2024
@mkckr0
Copy link
Owner

mkckr0 commented Apr 11, 2024

Thanks for your contribution!

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

2 participants