-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Explicitly add BUILD_TESTING CMake option #5627
Explicitly add BUILD_TESTING CMake option #5627
Conversation
@@ -17,6 +17,8 @@ option(BUILD_SHARED_LIBS "Whether to build shared libraries (like *.dll or *.so) | |||
|
|||
option(ADD_HEADERS_AS_SOURCES "Whether to add headers as sources of a target or not. This is needed for some IDEs which wouldn't detect headers properly otherwise") | |||
|
|||
option(BUILD_TESTING "Whether to enable and build tests or not") |
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 this also have a default of OFF
?
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.
If the starting value is not specified, the default is OFF.
Or If my first interpretation isn't right, it's normal to keep it off, because it yields to the fastest build for end users and developers.
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 like the implicitness explicitness of having a default
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 like the implicitness of having a default
Do you mean explicitness?
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.
hahahaha. yes.
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 thought it would've been redundant since you don't have multiple defaults to take into account (and this default is sane).
That been said, I'll merge this and do those changes in another PR, where I'm doing some small cleanups to variables.
@@ -17,6 +17,8 @@ option(BUILD_SHARED_LIBS "Whether to build shared libraries (like *.dll or *.so) | |||
|
|||
option(ADD_HEADERS_AS_SOURCES "Whether to add headers as sources of a target or not. This is needed for some IDEs which wouldn't detect headers properly otherwise") | |||
|
|||
option(BUILD_TESTING "Whether to enable and build tests or not") |
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 like the implicitness explicitness of having a default
This way is visible as a variable in the cache that can be set.
828f6c6
d3870ab
to
828f6c6
Compare
Rebased onto master. |
This way is visible as a variable in the cache that can be set.