Skip to content

Remove unused xml lib, update catch2 - #21

Merged
johguenther merged 2 commits into
develfrom
guj/devel
Sep 9, 2026
Merged

Remove unused xml lib, update catch2#21
johguenther merged 2 commits into
develfrom
guj/devel

Conversation

@johguenther

Copy link
Copy Markdown
Contributor

No description provided.

@johguenther johguenther changed the title Guj/devel Remove unused xml lib, update catch2 Sep 9, 2026
@johguenther
johguenther requested a lite review from Copilot September 9, 2026 13:20

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

It introduces an API-breaking removal (XML header) without an explicit breaking-change strategy, and the Catch2 tag is set in a way that prevents user override via CMake cache.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR updates the test infrastructure to fetch and build Catch2 v3 via CMake FetchContent, and removes the legacy rkcommon XML parsing/writing module from the library.

Changes:

  • Add a dedicated tests/external/catch2 FetchContent setup and link rkcommon_test_suite against Catch2::Catch2.
  • Update the test runner to use Catch::Session() (Catch2 v3).
  • Remove rkcommon/xml/XML.{h,cpp} from the project and update docs/build metadata accordingly.
File summaries
File Description
tests/external/catch2/CMakeLists.txt New FetchContent-based Catch2 v3 integration for tests.
tests/CMakeLists.txt Adds Catch2 subdir and links test suite against Catch2 target.
tests/catch_main.cpp Migrates test runner to Catch2 v3 session API.
rkcommon/xml/XML.h Removes legacy XML public header.
rkcommon/xml/XML.cpp Removes legacy XML implementation.
rkcommon/CMakeLists.txt Drops XML source file from rkcommon library build.
README.md Documents higher CMake requirement when building tests.
CMakeLists.txt Pins Catch2 git tag used by FetchContent.
Review details
  • Files reviewed: 8/9 changed files
  • Comments generated: 3
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread rkcommon/CMakeLists.txt
Comment thread CMakeLists.txt
Comment thread README.md Outdated
@johguenther
johguenther merged commit d191ca4 into devel Sep 9, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants