fgerlits commented on code in PR #1968:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1968#discussion_r2244839391
##########
CMakeLists.txt:
##########
@@ -316,8 +316,12 @@ else()
if(MINIFI_FAIL_ON_WARNINGS)
list(APPEND MINIFI_CPP_COMPILE_OPTIONS -Werror)
# -Wrestrict may cause build failure in GCC 12
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104336
- if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU")
- list(APPEND MINIFI_CPP_COMPILE_OPTIONS -Wno-error=restrict)
+ endif()
+ if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU")
+ list(APPEND MINIFI_CPP_COMPILE_OPTIONS -Wno-error=restrict)
Review Comment:
Can we remove `-Wno-error=restrict`? The bug linked to in line 318 was fixed
in GCC 12.4; I don't think we need to support GCC 12.1 to 12.3.
##########
.github/workflows/compiler-support.yml:
##########
@@ -0,0 +1,52 @@
+# .github/workflows/compiler-support.yml
+
+name: 'Check supported Compilers'
+
+on:
+ workflow_dispatch:
Review Comment:
Is the colon at the end of line 6 correct? I would expect
```suggestion
on: [workflow_dispatch]
```
##########
libminifi/test/unit/ExpectedTest.cpp:
##########
@@ -558,6 +558,15 @@ TEST_CASE("expected orThrow") {
nonstd::expected<int, std::string> unexpected{nonstd::unexpect, "hello"};
nonstd::expected<int, std::string> expected{5};
+
REQUIRE_THROWS_WITH(std::move(unexpected) | utils::orThrow("should throw"),
"should throw, but got hello");
CHECK((expected | utils::orThrow("should be 5")) == 5);
}
+
+// This fails to compile with std::expected on GCC 15.1 due to
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=119714
+TEST_CASE("") {
Review Comment:
Let's give this test a name; for example, the comment would work:
```suggestion
TEST_CASE("This fails to compile with std::expected on GCC 15.1 due to
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=119714";) {
```
##########
extensions/sql/processors/ExecuteSQL.cpp:
##########
@@ -56,7 +56,8 @@ void ExecuteSQL::processOnTrigger(core::ProcessContext&
context, core::ProcessSe
"No incoming FlowFile and the \"" +
std::string{SQLSelectQuery.name} + "\" processor property is not specified");
// NOLINT(whitespace/braces)
}
logger_->log_debug("Using the contents of the flow file as the SQL
statement");
- query = to_string(session.readBuffer(input_flow_file));
+ std::string buffer_str = to_string(session.readBuffer(input_flow_file));
+ query = buffer_str;
Review Comment:
what was the problem here?
##########
cmake/ExpectedLite.cmake:
##########
@@ -18,7 +18,10 @@
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
)
+
+add_compile_definitions(nsel_CONFIG_SELECT_EXPECTED=1) # Due to
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=119714
Review Comment:
Is this the same as
```suggestion
# Due to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=119714
add_compile_definitions(nsel_CONFIG_SELECT_EXPECTED=nsel_EXPECTED_NONSTD)
```
? I think that would be more readable than "1".
--
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]
