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]