-
Notifications
You must be signed in to change notification settings - Fork 48
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
[Work in Progress] Enable CMake build on Windows #1011
base: main
Are you sure you want to change the base?
Conversation
@@ -295,7 +322,7 @@ endif() | |||
# Packaging | |||
################################################################################################ | |||
|
|||
if (BUILD_PACKAGE) | |||
if (BUILD_PACKAGE AND (NOT WIN32)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor nit: what is the reason to check for NOT WIN32
- was it failing something if you don't add it? I wonder if we could eventually add logic in here under the if (BUILD_PACKAGE)
to produce a package, e.g. nuget or alike, even if we build on Windows. My selfish interest - is we'd likely need this CMake flow for our organization telemetry.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should also try to get Apple to consume vcpkgs as well, especially gtest/gmock. That can be a follow-up.
${CMAKE_CURRENT_SOURCE_DIR}/../third_party/googletest/googletest/include | ||
${CMAKE_CURRENT_SOURCE_DIR}/../third_party/googletest/googlemock/include | ||
) | ||
message("-->Lalit tests") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove?
|
||
PS C:\cpp_client_telemetry\build> cmake --build . | ||
|
||
This should build the 1DS C++ library, along with the functional and unit tests in `build` folder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add instructions on how to add it to another projects CMake as well? Like add_directory(<path to cpp_client_telemetry>)
?
Work in progress.