szaszm commented on code in PR #1968: URL: https://github.com/apache/nifi-minifi-cpp/pull/1968#discussion_r2282314433
########## cmake/ExpectedLite.cmake: ########## @@ -18,7 +18,11 @@ include(FetchContent) FetchContent_Declare(expected-lite - URL https://github.com/martinmoene/expected-lite/archive/refs/tags/v0.8.0.tar.gz - URL_HASH SHA256=27649f30bd9d4fe7b193ab3eb6f78c64d0f585c24c085f340b4722b3d0b5e701 + URL https://github.com/martinmoene/expected-lite/archive/refs/tags/v0.9.0.tar.gz + URL_HASH SHA256=e1b3ac812295ef8512c015d8271204105a71957323f8ab4e75f6856d71b8868d + SYSTEM ) + +# Due to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=119714 +add_compile_definitions(nsel_CONFIG_SELECT_EXPECTED=nsel_EXPECTED_NONSTD) Review Comment: Please change this to use target-based commands, i.e. target_compile_definitions. We should never use legacy global / directory-based cmake commands! ########## .dockerignore: ########## @@ -64,3 +64,5 @@ libminifi/src/agent/agent_version.cpp **/cmake_install.cmake **/install_manifest.txt **/CTestTestfile.cmake +bootstrap/venv +bootstrap/option_state.json Review Comment: ```suggestion bootstrap/option_state.json ``` ########## libminifi/test/unit/ExpectedTest.cpp: ########## @@ -473,7 +473,7 @@ TEST_CASE("expected valueOrElse", "[expected][valueOrElse]") { REQUIRE(gsl::narrow<int>("hello"sv.size()) == (ex | utils::valueOrElse([](const std::string& err) { return gsl::narrow<int>(err.size()); }))); REQUIRE_THROWS_AS(ex | utils::valueOrElse([](std::string){ throw std::exception(); }), std::exception); // NOLINT(performance-unnecessary-value-param) REQUIRE_THROWS_AS(ex | utils::valueOrElse([](const std::string&) -> int { throw std::exception(); }), std::exception); - REQUIRE_THROWS_WITH(std::move(ex) | utils::valueOrElse([](std::string&& error) -> int { throw std::runtime_error(error); }), "hello"); + REQUIRE_THROWS_WITH([&] { std::move(ex) | utils::valueOrElse([](const std::string& error) -> int { throw std::runtime_error(error); }); }(), "hello"); Review Comment: Why is the string taken by const ref now? The point of the test is to ensure that when an expected is moved, the callback gets an rvalue. ########## thirdparty/iODBC/GCC-15-needs-typedef-SQLRETURN-HPROC.patch: ########## @@ -0,0 +1,25 @@ +From 2d9ed2ef9d1cc320df388235356574890d7046d6 Mon Sep 17 00:00:00 2001 +From: Martin Zink <[email protected]> +Date: Tue, 22 Jul 2025 13:28:10 +0200 +Subject: [PATCH] GCC 15 needs typedef SQLRETURN (* HPROC) (...); + +--- + iodbc/dlproc.h | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/iodbc/dlproc.h b/iodbc/dlproc.h +index be2062b..ed15754 100644 +--- a/iodbc/dlproc.h ++++ b/iodbc/dlproc.h +@@ -80,7 +80,7 @@ + + #include <dlf.h> + +-#if defined(_MAC) || defined (__cplusplus) ++#if defined(_MAC) || defined (__cplusplus) || (defined(__GNUC__) && __GNUC__ >= 15) + typedef SQLRETURN (* HPROC) (...); + #else + typedef SQLRETURN (* HPROC) (); +-- +2.50.1 + Review Comment: Is this valid? ########## thirdparty/iODBC/GCC-15-needs-typedef-SQLRETURN-HPROC.patch: ########## Review Comment: did you submit this upstream? ########## thirdparty/couchbase/remove-thirdparty.patch: ########## @@ -76,7 +60,89 @@ index f02af02..f83c181 100644 "CMAKE_C_VISIBILITY_PRESET hidden" "CMAKE_CXX_VISIBILITY_PRESET hidden" "CMAKE_POSITION_INDEPENDENT_CODE ON") -@@ -159,93 +116,24 @@ if(NOT TARGET taocpp::json) + endif() Review Comment: this extra context seems unnecessary for the patch -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
