Skip to content

Commit

Permalink
Apply suggestions from code review
Browse files Browse the repository at this point in the history
Co-authored-by: Zachery Crandall <[email protected]>
  • Loading branch information
ryanmrichard and zachcran authored Dec 22, 2023
1 parent 4fa9124 commit 0efa63f
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ include(cmakepp_lang/cmakepp_lang)
#
# The various ``optional_dependency`` functions are tied to a configuration flag
# which determines whether the option is enabled or not. Users need to provide
# those functions with the name of the flag NOT its value. This function wraps
# the error-checking logic for ensuring that users passsed the value.
# those functions with the name of the flag, NOT its value. This function wraps
# the error-checking logic for ensuring that users passed the value.
#
# :param flag: Should be the name of a variable.
# :param flag: Name of the option variable.
# :type flag: desc
#
# :raises RUNTIME_ERROR: If ``flag`` is not the name of a variable. In
Expand All @@ -38,13 +38,12 @@ function(_check_optional_flag _cof_flag)
if("${_cof_flag}" STREQUAL "")
cpp_raise(
RUNTIME_ERROR
"Expected variable serving as the flag, recived an empty string"

"Expected variable serving as the flag, received an empty string"
)
elseif("${_cof_type}" STREQUAL "bool")
cpp_raise(
RUNTIME_ERROR
"Expected variable serving as the flag, recieved boolean value."
"Expected variable serving as the flag, received boolean value."
)
endif()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,13 @@ function("${test_check_optional_flag}")

endfunction()

ct_add_section(NAME "flag_value")
function("${flag_value}")

set(ENABLE_DEPENDENCY ON)
_check_optional_flag("${ENABLE_DEPENDENCY}")

endfunction()

ct_add_section(NAME "valid_flag")
function("${valid_flag}")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ function("${test_find_optional_dependency}")

find_python(py_exe py_version)
create_virtual_env(
venv_dir "${py_exe}" "${CMAKE_BINARY_DIR}" "${find_optional_dependency}"
venv_dir "${py_exe}" "${CMAKE_BINARY_DIR}" "${test_find_optional_dependency}"
)

# Make sure everything is using the venv Python
Expand Down

0 comments on commit 0efa63f

Please sign in to comment.