Skip to content

Conversation

@lordgamez
Copy link
Contributor

@lordgamez lordgamez commented Jul 29, 2025

  • Upgrade pugixml library to v1.15

https://issues.apache.org/jira/browse/MINIFICPP-2594

Depends on #1999


Thank you for submitting a contribution to Apache NiFi - MiNiFi C++.

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

For all changes:

  • Is there a JIRA ticket associated with this PR? Is it referenced
    in the commit message?

  • Does your PR title start with MINIFICPP-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.

  • Has your PR been rebased against the latest commit within the target branch (typically main)?

  • Is your initial contribution a single, squashed commit?

For code changes:

  • If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
  • If applicable, have you updated the LICENSE file?
  • If applicable, have you updated the NOTICE file?

For documentation related changes:

  • Have you ensured that format looks appropriate for the output in which it is rendered?

Note:

Please ensure that once the PR is submitted, you check GitHub Actions CI results for build issues and submit an update to your PR as soon as possible.

@lordgamez lordgamez force-pushed the MINIFICPP-2594 branch 2 times, most recently from 763701f to e6e8d8a Compare July 29, 2025 15:01
@lordgamez lordgamez marked this pull request as ready for review July 29, 2025 15:03
@lordgamez lordgamez changed the base branch from MINIFICPP-2587 to MINIFICPP-2600 August 6, 2025 08:47
@lordgamez lordgamez requested a review from Copilot August 14, 2025 11:41
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds XMLReader controller service functionality to Apache NiFi MiNiFi C++ and upgrades the pugixml library from v1.9 to v1.15.

  • Implements a new XMLReader controller service for parsing XML content into Record objects
  • Upgrades pugixml library to v1.15 and modernizes the build system to use FetchContent instead of ExternalProject
  • Updates build configurations to use the new pugixml library target

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
extensions/standard-processors/controllers/XMLReader.h Defines XMLReader controller service class with properties for XML parsing configuration
extensions/standard-processors/controllers/XMLReader.cpp Implements XML parsing logic and record creation functionality
extensions/standard-processors/tests/unit/XMLReaderTests.cpp Comprehensive test suite covering various XML parsing scenarios
cmake/PugiXml.cmake New CMake module for fetching pugixml v1.15 using FetchContent
cmake/BundledPugiXml.cmake Removes old ExternalProject-based pugixml build configuration
extensions/standard-processors/CMakeLists.txt Updates to link against new pugixml target
extensions/windows-event-log/CMakeLists.txt Updates WEL extension to use new pugixml target
CMakeLists.txt Includes new PugiXml module for standard processors and WEL extension
CONTROLLERS.md Documents the new XMLReader controller service properties

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

Copy link
Contributor

@fgerlits fgerlits left a comment

Choose a reason for hiding this comment

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

I think we need to add pugixml to the NOTICE file, as well.

Also, TinyXml2 is listed in both the LICENCE and NOTICE files, but we don't use it (any more?) -- it can be removed.

@lordgamez lordgamez changed the base branch from MINIFICPP-2600 to main September 9, 2025 07:10
@fgerlits fgerlits closed this in f28961c Sep 15, 2025
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.

4 participants