From dd6dd594252fdc576b74c19ea5f526dd0aae37f3 Mon Sep 17 00:00:00 2001 From: "Roscoe A. Bartlett" Date: Mon, 12 Dec 2022 13:29:50 -0700 Subject: [PATCH] Address review feedback from PR #546 (#63) This was feedback from @KyleFromKitware. * Fixed spelling of tribits_create_reverse_list() * Use lower-case set() * Put "${...}" around var name var in if ( ... "${${nameOfVar}}" STREQUAL "") to avoid confusion Run 'git log -p -1 --word-diff-regex=.' to best see what changed (and motivation for the changes should be obvious). --- test/core/TestingFunctionMacro_UnitTests.cmake | 14 +++++++------- .../TribitsWriteXmlDependenciesFiles.cmake | 4 ++-- .../package_arch/TribitsAdjustPackageEnables.cmake | 9 ++++----- tribits/core/utils/TribitsCreateReverseList.cmake | 6 +++--- .../doc/guides/UtilsMacroFunctionDocTemplate.rst | 2 +- 5 files changed, 17 insertions(+), 18 deletions(-) diff --git a/test/core/TestingFunctionMacro_UnitTests.cmake b/test/core/TestingFunctionMacro_UnitTests.cmake index 8261e208e..059276a67 100644 --- a/test/core/TestingFunctionMacro_UnitTests.cmake +++ b/test/core/TestingFunctionMacro_UnitTests.cmake @@ -133,21 +133,21 @@ function(unittest_append_string_var) endfunction() -function(unittest_tribit_create_reverse_list) +function(unittest_tribits_create_reverse_list) message("\n***") - message("*** Testing tribit_create_reverse_list()") + message("*** Testing tribits_create_reverse_list()") message("***\n") - message("tribit_create_reverse_list() with empty list") + message("tribits_create_reverse_list() with empty list") set(someEmptyList "") - tribit_create_reverse_list(someEmptyList someEmptyList_reversed) + tribits_create_reverse_list(someEmptyList someEmptyList_reversed) unittest_compare_const(someEmptyList_reversed "") - message("tribit_create_reverse_list() with non-empty list") + message("tribits_create_reverse_list() with non-empty list") set(someList a b c d) - tribit_create_reverse_list(someList someList_reversed) + tribits_create_reverse_list(someList someList_reversed) unittest_compare_const(someList_reversed "d;c;b;a") endfunction() @@ -4761,7 +4761,7 @@ message("*** Testing misc TriBITS functions and macros") message("***\n") unittest_append_string_var() -unittest_tribit_create_reverse_list() +unittest_tribits_create_reverse_list() unittest_tribits_find_python_interp() unittest_tribits_standardize_abs_paths() unittest_tribits_dir_is_basedir() diff --git a/tribits/ci_support/TribitsWriteXmlDependenciesFiles.cmake b/tribits/ci_support/TribitsWriteXmlDependenciesFiles.cmake index b59f047ac..1091008e3 100644 --- a/tribits/ci_support/TribitsWriteXmlDependenciesFiles.cmake +++ b/tribits/ci_support/TribitsWriteXmlDependenciesFiles.cmake @@ -220,14 +220,14 @@ function(tribits_get_legacy_package_deps_sublist packageName libOrTest set(matchesRequriedOrOptional TRUE) endif() - SET(matchesPackagesOrTpls FALSE) + set(matchesPackagesOrTpls FALSE) if (((packagesOrTpls STREQUAL "PACKAGES") AND (${depPkg}_PACKAGE_BUILD_STATUS STREQUAL "INTERNAL")) OR ((packagesOrTpls STREQUAL "TPLS") AND (${depPkg}_PACKAGE_BUILD_STATUS STREQUAL "EXTERNAL")) ) - SET(matchesPackagesOrTpls TRUE) + set(matchesPackagesOrTpls TRUE) endif() if (matchesRequriedOrOptional AND matchesPackagesOrTpls) diff --git a/tribits/core/package_arch/TribitsAdjustPackageEnables.cmake b/tribits/core/package_arch/TribitsAdjustPackageEnables.cmake index 6de659d6c..53bbb1f2b 100644 --- a/tribits/core/package_arch/TribitsAdjustPackageEnables.cmake +++ b/tribits/core/package_arch/TribitsAdjustPackageEnables.cmake @@ -172,7 +172,7 @@ macro(tribits_sweep_forward_apply_enables) tribits_get_sublist_nondisabled( ${PROJECT_NAME}_DEFINED_INTERNAL_PACKAGES ${PROJECT_NAME}_NOTDISABLED_INTERNAL_PACKAGES "") - tribit_create_reverse_list(${PROJECT_NAME}_NOTDISABLED_INTERNAL_PACKAGES + tribits_create_reverse_list(${PROJECT_NAME}_NOTDISABLED_INTERNAL_PACKAGES ${PROJECT_NAME}_REVERSE_NOTDISABLED_INTERNAL_PACKAGES) message("\nEnabling subpackages for hard enables of parent packages" @@ -200,7 +200,7 @@ macro(tribits_sweep_forward_apply_enables) tribits_get_sublist_enabled( ${PROJECT_NAME}_DEFINED_INTERNAL_PACKAGES ${PROJECT_NAME}_ENABLED_INTERNAL_PACKAGES "") - tribit_create_reverse_list(${PROJECT_NAME}_ENABLED_INTERNAL_PACKAGES + tribits_create_reverse_list(${PROJECT_NAME}_ENABLED_INTERNAL_PACKAGES ${PROJECT_NAME}_REVERSE_ENABLED_INTERNAL_PACKAGES) message("\nSweep backward enabling all forward test dependent packages because" @@ -295,7 +295,7 @@ macro(tribits_sweep_backward_enable_upstream_packages) tribits_get_sublist_nondisabled( ${PROJECT_NAME}_DEFINED_PACKAGES ${PROJECT_NAME}_NOTDISABLED_PACKAGES "") - tribit_create_reverse_list(${PROJECT_NAME}_NOTDISABLED_PACKAGES + tribits_create_reverse_list(${PROJECT_NAME}_NOTDISABLED_PACKAGES ${PROJECT_NAME}_REVERSE_NOTDISABLED_PACKAGES) message("\nEnabling all required${tap1_extraMsgStr} upstream packages for current" @@ -893,7 +893,7 @@ macro(tribits_private_disable_required_package_enables fwdDepPkgName packageName libraryDep ) tribits_get_package_enable_status(${fwdDepPkgName} "" fwdDepPkgEnableVarName) - if (${fwdDepPkgEnableVarName} OR ${fwdDepPkgEnableVarName} STREQUAL "") + if (${fwdDepPkgEnableVarName} OR "${${fwdDepPkgEnableVarName}}" STREQUAL "") if ("${libraryDep}" STREQUAL "TRUE") tribits_private_print_disable_required_package_enable( ${packageName} ${fwdDepPkgEnableVarName} @@ -922,7 +922,6 @@ macro(tribits_private_disable_required_package_enables endif() endmacro() - function(tribits_private_print_disable_required_package_enable packageName packageEnableSomethingVarName fwdDepPkgName depTypeStr diff --git a/tribits/core/utils/TribitsCreateReverseList.cmake b/tribits/core/utils/TribitsCreateReverseList.cmake index 5f50d59ad..2f84d8c47 100644 --- a/tribits/core/utils/TribitsCreateReverseList.cmake +++ b/tribits/core/utils/TribitsCreateReverseList.cmake @@ -38,15 +38,15 @@ # @HEADER -# @MACRO: tribit_create_reverse_list() +# @MACRO: tribits_create_reverse_list() # # Create a reverse list var in one shot. # # Usage:: # -# tribit_create_reverse_list( ) +# tribits_create_reverse_list( ) # -macro(tribit_create_reverse_list oldListName newListName) +macro(tribits_create_reverse_list oldListName newListName) set(${newListName} ${${oldListName}}) list(REVERSE ${newListName}) endmacro() diff --git a/tribits/doc/guides/UtilsMacroFunctionDocTemplate.rst b/tribits/doc/guides/UtilsMacroFunctionDocTemplate.rst index 3d6140dc7..d7d18c8d7 100644 --- a/tribits/doc/guides/UtilsMacroFunctionDocTemplate.rst +++ b/tribits/doc/guides/UtilsMacroFunctionDocTemplate.rst @@ -42,7 +42,7 @@ @FUNCTION: tribits_add_enum_cache_var() + @FUNCTION: tribits_deprecated() + @FUNCTION: tribits_deprecated_command() + -@MACRO: tribit_create_reverse_list() + +@MACRO: tribits_create_reverse_list() + @FUNCTION: tribits_strip_quotes_from_str() + @FUNCTION: unittest_compare_const() + @FUNCTION: unittest_has_substr_const() +