Skip to content

Commit c5426d7

Browse files
leftibotclaude
andcommitted
Address review: remove CMake test scripts, don't SYSTEM-include lefticus tools
Remove check_system_includes.cmake.in and associated test from test/CMakeLists.txt — these would confuse end users. Remove SYSTEM YES from the lefticus/tools CPMAddPackage call so its headers are not treated as system includes. Requested by @lefticus in PR #145 review. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 528bdb7 commit c5426d7

3 files changed

Lines changed: 1 addition & 30 deletions

File tree

Dependencies.cmake

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,7 @@ function(myproject_setup_dependencies)
7777
GITHUB_REPOSITORY
7878
"lefticus/tools"
7979
GIT_TAG
80-
"main"
81-
SYSTEM
82-
YES)
80+
"main")
8381
endif()
8482

8583
endfunction()

test/CMakeLists.txt

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -21,20 +21,6 @@ endif()
2121

2222
include(${Catch2_SOURCE_DIR}/extras/Catch.cmake)
2323

24-
# Regression test for issue #143: verify CPM dependencies use SYSTEM includes
25-
get_property(_fmt_dir TARGET fmt::fmt PROPERTY SOURCE_DIR)
26-
if(_fmt_dir)
27-
get_property(_fmt_dir_system DIRECTORY "${_fmt_dir}" PROPERTY SYSTEM)
28-
else()
29-
set(_fmt_dir_system FALSE)
30-
endif()
31-
configure_file(
32-
"${CMAKE_CURRENT_SOURCE_DIR}/check_system_includes.cmake.in"
33-
"${CMAKE_CURRENT_BINARY_DIR}/check_system_includes.cmake"
34-
@ONLY)
35-
add_test(NAME cmake.system_includes
36-
COMMAND ${CMAKE_COMMAND} -P "${CMAKE_CURRENT_BINARY_DIR}/check_system_includes.cmake")
37-
3824
# Provide a simple smoke test to make sure that the CLI works and can display a --help message
3925
add_test(NAME cli.has_help COMMAND intro --help)
4026

test/check_system_includes.cmake.in

Lines changed: 0 additions & 13 deletions
This file was deleted.

0 commit comments

Comments
 (0)