Skip to content

Commit 9a4099c

Browse files
davidliondavidlion
committed
Address review comments.
1 parent fc773c1 commit 9a4099c

4 files changed

Lines changed: 48 additions & 44 deletions

File tree

‎CMakeLists.txt‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -85,24 +85,27 @@ if(ystdlib_ENABLE_TESTS)
8585
catch_discover_tests(${UNIFIED_UNIT_TEST_TARGET} WORKING_DIRECTORY ${CMAKE_BINARY_DIR}/testbin)
8686
endif()
8787

88-
set(ystdlib_INSTALL_CONFIG_DIR ${CMAKE_INSTALL_LIBDIR}/cmake/ystdlib)
88+
# All libraries are required to use the same minimum version of dependencies to avoid issues.
89+
set(MIN_BOOST_VERSOIN "1.81.0")
8990

9091
add_subdirectory(src/ystdlib)
9192

93+
set(ystdlib_INSTALL_CONFIG_DIR ${CMAKE_INSTALL_LIBDIR}/cmake/ystdlib)
94+
95+
set(CONFIG_FILE_PREFIX "ystdlib-config")
96+
set(CONFIG_OUTPUT_PATH "${CMAKE_CURRENT_BINARY_DIR}/${CONFIG_FILE_PREFIX}.cmake")
97+
set(CONFIG_VERSION_OUTPUT_PATH "${CMAKE_CURRENT_BINARY_DIR}/${CONFIG_FILE_PREFIX}-version.cmake")
9298
configure_package_config_file(
93-
${CMAKE_CURRENT_LIST_DIR}/cmake/ystdlib-config.cmake.in
94-
${CMAKE_CURRENT_BINARY_DIR}/ystdlib-config.cmake
95-
INSTALL_DESTINATION ${ystdlib_INSTALL_CONFIG_DIR}
99+
"${CMAKE_CURRENT_LIST_DIR}/cmake/${CONFIG_FILE_PREFIX}.cmake.in"
100+
"${CONFIG_OUTPUT_PATH}"
101+
INSTALL_DESTINATION "${ystdlib_INSTALL_CONFIG_DIR}"
96102
)
97103

98-
write_basic_package_version_file(
99-
${CMAKE_CURRENT_BINARY_DIR}/ystdlib-config-version.cmake
100-
COMPATIBILITY SameMajorVersion
101-
)
104+
write_basic_package_version_file("${CONFIG_VERSION_OUTPUT_PATH}" COMPATIBILITY SameMajorVersion)
102105

103106
install(
104107
FILES
105-
${CMAKE_CURRENT_BINARY_DIR}/ystdlib-config.cmake
106-
${CMAKE_CURRENT_BINARY_DIR}/ystdlib-config-version.cmake
107-
DESTINATION ${ystdlib_INSTALL_CONFIG_DIR}
108+
"${CONFIG_OUTPUT_PATH}"
109+
"${CONFIG_VERSION_OUTPUT_PATH}"
110+
DESTINATION "${ystdlib_INSTALL_CONFIG_DIR}"
108111
)

‎cmake/ystdlib-helpers.cmake‎

Lines changed: 22 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,11 @@ include(CMakePackageConfigHelpers)
44
# arguments are stored in variables prefixed with `ARG_<NAME>`.
55
#
66
# @param {string[]} REQUIRED_ARG_NAMES
7-
macro(require_argument_values REQUIRED_ARG_NAMES)
8-
set(_REQUIRED_ARGS "${REQUIRED_ARG_NAMES}")
9-
foreach(_REQUIRED_ARG IN LISTS _REQUIRED_ARGS)
10-
if(NOT DEFINED ARG_${_REQUIRED_ARG} OR ARG_${_REQUIRED_ARG} STREQUAL "")
11-
message(FATAL_ERROR "Empty value for argument: '${_REQUIRED_ARG}'")
7+
macro(check_required_arguments_exist REQUIRED_ARG_NAMES)
8+
set(_NAMES "${REQUIRED_ARG_NAMES}")
9+
foreach(_NAME IN LISTS _NAMES)
10+
if(NOT DEFINED ARG_${_NAME} OR ARG_${_NAME} STREQUAL "")
11+
message(FATAL_ERROR "Empty value for argument: '${_NAME}'")
1212
endif()
1313
endforeach()
1414
endmacro()
@@ -60,7 +60,7 @@ function(add_cpp_library)
6060
PUBLIC_HEADERS
6161
)
6262
cmake_parse_arguments(ARG "" "${SINGLE_VALUE_ARGS}" "${MULTI_VALUE_ARGS}" ${ARGN})
63-
require_argument_values("${REQUIRED_ARGS}")
63+
check_required_arguments_exist("${REQUIRED_ARGS}")
6464

6565
if(NOT DEFINED ARG_BUILD_INCLUDE_DIRS)
6666
set(ARG_BUILD_INCLUDE_DIRS "${PROJECT_SOURCE_DIR}/src")
@@ -139,7 +139,7 @@ function(add_catch2_tests)
139139
SOURCES
140140
)
141141
cmake_parse_arguments(ARG "" "${SINGLE_VALUE_ARGS}" "${MULTI_VALUE_ARGS}" ${ARGN})
142-
require_argument_values("${REQUIRED_ARGS}")
142+
check_required_arguments_exist("${REQUIRED_ARGS}")
143143

144144
set(ALIAS_TARGET "${ARG_NAMESPACE}::${ARG_NAME}")
145145
set(UNIT_TEST_TARGET "unit-test-${ARG_NAME}")
@@ -150,8 +150,8 @@ function(add_catch2_tests)
150150
${UNIT_TEST_TARGET}
151151
PRIVATE
152152
Catch2::Catch2WithMain
153-
${ALIAS_TARGET}
154153
${ARG_LINK_LIBRARIES}
154+
${ALIAS_TARGET}
155155
)
156156
target_compile_features(${UNIT_TEST_TARGET} PRIVATE cxx_std_20)
157157
set_property(
@@ -167,8 +167,8 @@ function(add_catch2_tests)
167167
target_link_libraries(
168168
${ARG_UNIFIED_TEST_TARGET}
169169
PRIVATE
170-
${ALIAS_TARGET}
171170
${ARG_LINK_LIBRARIES}
171+
${ALIAS_TARGET}
172172
)
173173
endif()
174174
endfunction()
@@ -177,12 +177,12 @@ endfunction()
177177
#
178178
# @param {string} NAME
179179
# @param {string} NAMESPACE
180-
# @param {string} [CONFIG_DEST_DIR] Destination to install the generated config file
181-
# (`NAME-config.cmake`).
182-
# @param {string} [CONFIG_INPUT_DIR] `configure_package_config_file` input file
183-
# (`NAME-config.cmake.in`).
184-
# @param {string} [CONFIG_OUTPUT_DIR] `configure_package_config_file` output file
185-
# (`NAME-config.cmake`).
180+
# @param {string} [CONFIG_DEST_DIR="${CMAKE_INSTALL_LIBDIR}/cmake/${ARG_NAMESPACE}/libs"]
181+
# Destination to install the generated config file (`NAME-config.cmake`).
182+
# @param {string} [CONFIG_INPUT_DIR="${PROJECT_SOURCE_DIR}/cmake/libs"]
183+
# `configure_package_config_file` input file (`NAME-config.cmake.in`).
184+
# @param {string} [CONFIG_OUTPUT_DIR="${CMAKE_CURRENT_BINARY_DIR}"] `configure_package_config_file`
185+
# output file (`NAME-config.cmake`).
186186
function(install_library)
187187
set(SINGLE_VALUE_ARGS
188188
NAME
@@ -196,7 +196,7 @@ function(install_library)
196196
NAMESPACE
197197
)
198198
cmake_parse_arguments(ARG "" "${SINGLE_VALUE_ARGS}" "" ${ARGN})
199-
require_argument_values("${REQUIRED_ARGS}")
199+
check_required_arguments_exist("${REQUIRED_ARGS}")
200200

201201
if(NOT DEFINED ARG_CONFIG_DEST_DIR)
202202
set(ARG_CONFIG_DEST_DIR "${CMAKE_INSTALL_LIBDIR}/cmake/${ARG_NAMESPACE}/libs")
@@ -219,15 +219,13 @@ function(install_library)
219219
NAMESPACE "${ARG_NAMESPACE}::"
220220
)
221221

222+
set(CONFIG_FILE_NAME "${ARG_NAME}-config.cmake")
223+
set(CONFIG_OUTPUT_PATH "${ARG_CONFIG_OUTPUT_DIR}/${CONFIG_FILE_NAME}")
222224
configure_package_config_file(
223-
${ARG_CONFIG_INPUT_DIR}/${ARG_NAME}-config.cmake.in
224-
${ARG_CONFIG_OUTPUT_DIR}/${ARG_NAME}-config.cmake
225-
INSTALL_DESTINATION ${ARG_CONFIG_DEST_DIR}
225+
"${ARG_CONFIG_INPUT_DIR}/${CONFIG_FILE_NAME}.in"
226+
"${CONFIG_OUTPUT_PATH}"
227+
INSTALL_DESTINATION "${ARG_CONFIG_DEST_DIR}"
226228
)
227229

228-
install(
229-
FILES
230-
${ARG_CONFIG_OUTPUT_DIR}/${ARG_NAME}-config.cmake
231-
DESTINATION ${ARG_CONFIG_DEST_DIR}
232-
)
230+
install(FILES "${CONFIG_OUTPUT_PATH}" DESTINATION "${ARG_CONFIG_DEST_DIR}")
233231
endfunction()

‎examples/src/linking-tests.cpp‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,24 +44,29 @@ using ystdlib::io_interface::ErrorCode;
4444

4545
class FailureReader : public ystdlib::io_interface::ReaderInterface {
4646
public:
47-
[[nodiscard]] auto read(char* /*buf*/, size_t /*num_bytes_to_read*/, size_t& /*num_bytes_read*/)
47+
[[nodiscard]] auto read([[maybe_unused]] char* buf, [[maybe_unused]] size_t num_bytes_to_read, [[maybe_unused]] size_t& num_bytes_read)
4848
-> ErrorCode override {
4949
return ErrorCode::ErrorCode_Unsupported;
5050
}
5151

52-
[[nodiscard]] auto seek_from_begin(size_t /*pos*/) -> ErrorCode override {
52+
[[nodiscard]] auto seek_from_begin([[maybe_unused]] size_t pos) -> ErrorCode override {
5353
return ErrorCode::ErrorCode_Unsupported;
5454
}
5555

56-
[[nodiscard]] auto seek_from_current(off_t /*offset*/) -> ErrorCode override {
56+
[[nodiscard]] auto seek_from_current([[maybe_unused]] off_t offset) -> ErrorCode override {
5757
return ErrorCode::ErrorCode_Unsupported;
5858
}
5959

60-
[[nodiscard]] auto get_pos(size_t& /*pos*/) -> ErrorCode override {
60+
[[nodiscard]] auto get_pos([[maybe_unused]] size_t& pos) -> ErrorCode override {
6161
return ErrorCode::ErrorCode_Unsupported;
6262
}
6363
};
6464

65+
auto test_containers() -> bool;
66+
auto test_error_handling() -> bool;
67+
auto test_io_interface() -> bool;
68+
auto test_wrapped_facade_headers() -> bool;
69+
6570
auto test_containers() -> bool {
6671
try {
6772
constexpr size_t cBufferSize{1024};

‎src/ystdlib/error_handling/CMakeLists.txt‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,13 @@ if(TARGET ystdlib::error_handling)
22
return()
33
endif()
44

5-
set(boost_find_package_args
6-
1.81.0
5+
set(BOOST_FIND_PACKAGE_ARGS
6+
"${MIN_BOOST_VERSOIN}"
77
REQUIRED
88
COMPONENTS
99
headers
1010
)
11-
find_package(Boost ${boost_find_package_args})
11+
find_package(Boost ${BOOST_FIND_PACKAGE_ARGS})
1212
message(STATUS "Found Boost ${Boost_VERSION}.")
1313

1414
add_cpp_library(
@@ -28,12 +28,10 @@ if(ystdlib_ENABLE_TESTS)
2828
NAME error_handling
2929
NAMESPACE ystdlib
3030
SOURCES
31-
test/constants.hpp
3231
test/test_ErrorCode.cpp
3332
test/test_Result.cpp
3433
test/test_TraceableException.cpp
3534
test/types.cpp
36-
test/types.hpp
3735
UNIFIED_TEST_TARGET "${UNIFIED_UNIT_TEST_TARGET}"
3836
)
3937
endif()

0 commit comments

Comments
 (0)