-
Notifications
You must be signed in to change notification settings - Fork 5k
Fix cmake warning #116000
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 cmake warning #116000
Conversation
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.
Pull Request Overview
This PR updates the endif
comment in configureplatform.cmake
to match the full conditional and eliminate a CMake warning.
- Expanded the
endif
comment to include bothemscripten
andbrowser
. - Resolves mismatch between
if
andendif
conditions causing the warning.
Tagging subscribers to this area: @dotnet/runtime-infrastructure |
@@ -437,7 +437,7 @@ endif(CLR_CMAKE_TARGET_OS STREQUAL haiku) | |||
if(CLR_CMAKE_TARGET_OS STREQUAL emscripten OR CLR_CMAKE_TARGET_OS STREQUAL browser) | |||
set(CLR_CMAKE_TARGET_UNIX 1) | |||
set(CLR_CMAKE_TARGET_BROWSER 1) | |||
endif(CLR_CMAKE_TARGET_OS STREQUAL emscripten) | |||
endif(CLR_CMAKE_TARGET_OS STREQUAL emscripten OR CLR_CMAKE_TARGET_OS STREQUAL browser) |
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 can also change this to not have the condition repeated.
endif(CLR_CMAKE_TARGET_OS STREQUAL emscripten OR CLR_CMAKE_TARGET_OS STREQUAL browser) | |
endif() |
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.
Yes, but that would make it inconsistent with the rest of the file.
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.
I never liked the repeated condition but I agree this should be made in a different PR if we were to change it
/ba-g unrelated infrastructure issue |
Fixes #115998