szaszm commented on code in PR #2155:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2155#discussion_r3052696827


##########
core-framework/src/utils/Cron.cpp:
##########
@@ -46,14 +46,15 @@ namespace org::apache::nifi::minifi::utils {
 namespace {
 
 // https://github.com/HowardHinnant/date/issues/550
-// Due to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78714
-// the month parsing with '%b' and the weekday parsing with '%a' is 
case-sensitive in gcc11
-// This has been fixed in gcc13
+// Due to libstdc++ bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78714
+// the month parsing with '%b' and the weekday parsing with '%a' can be 
case-sensitive.
+// Normalizing to Title Case prevents parsing failures across all GCC versions.

Review Comment:
   If you want to drop GCC < 13, then we can drop the workaround too.



##########
libminifi/test/unit/ExpectedTest.cpp:
##########


Review Comment:
   the monadic operations are standard, we don't need these utilities or their 
tests anymore



##########
docker/rockylinux/Dockerfile:
##########
@@ -52,16 +52,13 @@ RUN cd $MINIFI_BASE_DIR && \
 RUN groupadd -g ${GID} ${USER} && useradd -g ${GID} ${USER} && \
     chown -R ${USER}:${USER} ${MINIFI_BASE_DIR}
 
-# Patching standard header to avoid 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105651
-RUN patch -p1 
/opt/rh/gcc-toolset-12/root/usr/include/c++/12/bits/basic_string.tcc 
${MINIFI_BASE_DIR}/thirdparty/libstdc++/avoid_bogus_Wrestrict_PR105651.patch

Review Comment:
   please remove the patch file too



##########
extensions/standard-processors/processors/InvokeHTTP.cpp:
##########
@@ -79,16 +79,16 @@ void HttpClientStore::returnClient(http::HTTPClient& 
client) {
 }  // namespace invoke_http
 
 namespace {
-nonstd::expected<std::string_view, std::error_code> removePerSecSuffix(const 
std::string_view input) {
+std::expected<std::string_view, std::error_code> removePerSecSuffix(const 
std::string_view input) {
   const auto trimmed_input = utils::string::trim(input);
   if (trimmed_input.ends_with("/s") || trimmed_input.ends_with("/S")) {
     return trimmed_input.substr(0, trimmed_input.size() - 2);
   }
-  return nonstd::make_unexpected(core::ParsingErrorCode::GeneralParsingError);
+  return std::unexpected(core::ParsingErrorCode::GeneralParsingError);
 }
 }  // namespace
 
-nonstd::expected<uint64_t, std::error_code> 
invoke_http::parseDataTransferSpeed(const std::string_view input) {
+std::expected<uint64_t, std::error_code> 
invoke_http::parseDataTransferSpeed(const std::string_view input) {

Review Comment:
   monadic operations are standard now, so we should replace utils::andThen 
with .and_then too



##########
libminifi/src/core/ProcessContextImpl.cpp:
##########
@@ -99,10 +99,10 @@ uint8_t ProcessContextImpl::getMaxConcurrentTasks() const { 
return processor_.ge
 
 void ProcessContextImpl::yield() { processor_.yield(); }
 
-nonstd::expected<std::string, std::error_code> 
ProcessContextImpl::getProperty(const std::string_view name, const FlowFile* 
flow_file) const {
+std::expected<std::string, std::error_code> 
ProcessContextImpl::getProperty(const std::string_view name, const FlowFile* 
flow_file) const {
   const auto property = getProcessorInfo().getSupportedProperty(name);
   if (!property) {
-    return nonstd::make_unexpected(PropertyErrorCode::NotSupportedProperty);
+    return std::unexpected(PropertyErrorCode::NotSupportedProperty);

Review Comment:
   I would use braced init with `std::unexpected` to make it clear that this is 
a constructor call, and for the other benefits of direct-list-initialization, 
e.g. no implicit narrowing. 
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es23-prefer-the--initializer-syntax



##########
.github/workflows/compiler-support.yml:
##########
@@ -11,7 +11,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        platform: [ { id: gcc-12, base_image: gcc:12-bookworm },
+        platform: [ { id: gcc-13, base_image: gcc:13-bookworm },

Review Comment:
   let's replace the 15 image tag with 15-trixie, the image authors seem to 
have moved on to debian trixie base with 14 and 15
   https://hub.docker.com/_/gcc



-- 
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]

Reply via email to