You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by "lordgamez (via GitHub)" <gi...@apache.org> on 2025/09/15 16:27:37 UTC
[PR] MINIFICPP-2629 Clean up minifi-api includes [nifi-minifi-cpp]
lordgamez opened a new pull request, #2029:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2029
- Remove duplicate header files with the same content
- Remove proxy header files in core/extension-framework that only include minifi-api headers
- Include only the minifi-api headers when implementation is not used
- Additionally fix clang-tidy issues in these files
https://issues.apache.org/jira/browse/MINIFICPP-2629
--------
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](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] 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.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2629 Clean up minifi-api includes [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #2029:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2029#discussion_r2372003220
##########
extensions/standard-processors/utils/JoltUtils.h:
##########
@@ -58,7 +58,7 @@ class Spec {
return nullptr;
}
- template<std::invocable<std::shared_ptr<core::logging::Logger>> OnEnterFn, std::invocable<std::shared_ptr<core::logging::Logger>> OnExitFn>
+ template<std::invocable<const std::shared_ptr<core::logging::Logger>&> OnEnterFn, std::invocable<const std::shared_ptr<core::logging::Logger>&> OnExitFn>
Review Comment:
This change doesn't touch the shared pointers, it just checks the interface of the callable. Whether it gets increased or not depends on the function signature, not the concept checks.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2629 Clean up minifi-api includes [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm closed pull request #2029: MINIFICPP-2629 Clean up minifi-api includes
URL: https://github.com/apache/nifi-minifi-cpp/pull/2029
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2629 Clean up minifi-api includes [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #2029:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2029#discussion_r2372016566
##########
extensions/systemd/ConsumeJournald.cpp:
##########
@@ -78,7 +78,7 @@ void ConsumeJournald::onSchedule(core::ProcessContext& context, core::ProcessSes
return process_old_messages ? journal.seekHead() : journal.seekTail();
};
worker_->enqueue([this, &seek_default] {
- const auto cursor = state_manager_->get() | utils::transform([](std::unordered_map<std::string, std::string>&& m) { return m.at(CURSOR_KEY); });
+ const auto cursor = state_manager_->get() | utils::transform([](const std::unordered_map<std::string, std::string>& m) { return m.at(CURSOR_KEY); });
Review Comment:
I forgot about the fact that the effective type of rvalue ref parameter variables within the function body is mutable lvalue refs. I guess it's fine either way.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2629 Clean up minifi-api includes [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #2029:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2029#discussion_r2368628272
##########
extensions/systemd/ConsumeJournald.cpp:
##########
@@ -78,7 +78,7 @@ void ConsumeJournald::onSchedule(core::ProcessContext& context, core::ProcessSes
return process_old_messages ? journal.seekHead() : journal.seekTail();
};
worker_->enqueue([this, &seek_default] {
- const auto cursor = state_manager_->get() | utils::transform([](std::unordered_map<std::string, std::string>&& m) { return m.at(CURSOR_KEY); });
+ const auto cursor = state_manager_->get() | utils::transform([](const std::unordered_map<std::string, std::string>& m) { return m.at(CURSOR_KEY); });
Review Comment:
why was this changed? we can capture a temporary by either reference type, but it's an rvalue, so an rvalue reference seems better fitting. It would allow moving out the cursor value if unordered_map had an rvalue qualified overload of at.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2629 Clean up minifi-api includes [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #2029:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2029#discussion_r2371999120
##########
extensions/sftp/processors/ListSFTP.cpp:
##########
@@ -690,7 +690,7 @@ void ListSFTP::listByTrackingEntities(
time_t now = time(nullptr);
uint64_t min_timestamp_to_list = (!initial_listing_complete_ && entity_tracking_initial_listing_target_ == ENTITY_TRACKING_INITIAL_LISTING_TARGET_ALL_AVAILABLE)
- ? 0U : (now * 1000 - entity_tracking_time_window.count());
+ ? 0U : ((now * 1000) - entity_tracking_time_window.count());
Review Comment:
I didn't realize this was another clang-tidy check. Yes, please disable it. Putting parens around everything makes no sense, precedence is a well known and useful thing.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2629 Clean up minifi-api includes [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #2029:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2029#discussion_r2439267095
##########
.clang-tidy:
##########
@@ -1,6 +1,7 @@
Checks: >
-clang-analyzer-optin.cplusplus.VirtualCall,
-clang-analyzer-optin.core.EnumCastOutOfRange,
+ -clang-analyzer-unix.BlockInCriticalSection,
Review Comment:
I've checked this: it warns for the read in FileMutex, and asio internals whenever networking is used. None of the warnings seemed useful: FileMutex is inherently meant to interact with files atomically (achieved with the mutex), and asio internal mutexes are not our responsibility.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2629 Clean up minifi-api includes [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #2029:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2029#discussion_r2368523396
##########
extensions/sftp/processors/ListSFTP.cpp:
##########
@@ -690,7 +690,7 @@ void ListSFTP::listByTrackingEntities(
time_t now = time(nullptr);
uint64_t min_timestamp_to_list = (!initial_listing_complete_ && entity_tracking_initial_listing_target_ == ENTITY_TRACKING_INITIAL_LISTING_TARGET_ALL_AVAILABLE)
- ? 0U : (now * 1000 - entity_tracking_time_window.count());
+ ? 0U : ((now * 1000) - entity_tracking_time_window.count());
Review Comment:
disagree with the extra parens: multiplication has higher precedence, it's not confusing, and it's on the left side anyway, so it's the same even in left to right order.
##########
extensions/systemd/ConsumeJournald.cpp:
##########
@@ -78,7 +78,7 @@ void ConsumeJournald::onSchedule(core::ProcessContext& context, core::ProcessSes
return process_old_messages ? journal.seekHead() : journal.seekTail();
};
worker_->enqueue([this, &seek_default] {
- const auto cursor = state_manager_->get() | utils::transform([](std::unordered_map<std::string, std::string>&& m) { return m.at(CURSOR_KEY); });
+ const auto cursor = state_manager_->get() | utils::transform([](const std::unordered_map<std::string, std::string>& m) { return m.at(CURSOR_KEY); });
Review Comment:
why was this changed? we can capture a temporary by either reference type, but it's an rvalue, so an rvalue reference seems better fitting. It would allow out the cursor value if unordered_map had an rvalue qualified overload of at.
##########
extensions/kafka/PublishKafka.cpp:
##########
@@ -112,7 +112,7 @@ class PublishKafka::Messages {
}
template<typename Func>
- auto modifyResult(size_t index, Func fun) -> decltype(fun(flow_files_.at(index))) {
+ auto modifyResult(size_t index, const Func& fun) -> decltype(fun(flow_files_.at(index))) {
Review Comment:
why was this changed? if there is no good justification, I'd change it back.
##########
extensions/standard-processors/processors/SplitText.cpp:
##########
@@ -53,7 +53,7 @@ uint8_t LineReader::getEndLineSize(size_t newline_position) {
}
void LineReader::setLastLineInfoAttributes(uint8_t endline_size, const std::optional<std::string>& starts_with) {
- const uint64_t size_from_beginning_of_stream = (current_buffer_count_ - 1) * buffer_size_ + buffer_offset_;
+ const uint64_t size_from_beginning_of_stream = ((current_buffer_count_ - 1) * buffer_size_) + buffer_offset_;
Review Comment:
same thing about parens here
##########
extensions/standard-processors/utils/JoltUtils.h:
##########
@@ -58,7 +58,7 @@ class Spec {
return nullptr;
}
- template<std::invocable<std::shared_ptr<core::logging::Logger>> OnEnterFn, std::invocable<std::shared_ptr<core::logging::Logger>> OnExitFn>
+ template<std::invocable<const std::shared_ptr<core::logging::Logger>&> OnEnterFn, std::invocable<const std::shared_ptr<core::logging::Logger>&> OnExitFn>
Review Comment:
disagree with this change: Whether the argument is const-ref or not doesn't make a difference from the standpoint of being able to call a function with it or not.
##########
libminifi/test/unit/ResponseNodeLoaderTests.cpp:
##########
@@ -49,7 +50,7 @@ class ResponseNodeLoaderTestFixture {
}
protected:
- template<typename T, typename = typename std::enable_if_t<std::is_base_of_v<minifi::core::ProcessorImpl, T>>>
+ template<typename T> requires(std::is_base_of_v<minifi::core::ProcessorImpl, T>)
Review Comment:
unnecessary parens
##########
extensions/standard-processors/processors/AttributeRollingWindow.cpp:
##########
@@ -113,7 +113,7 @@ void AttributeRollingWindow::calculateAndSetAttributes(core::FlowFile &flow_file
}());
// https://math.stackexchange.com/questions/1720876/sums-of-squares-minus-square-of-sums
const auto avg_of_squares = std::accumulate(std::begin(sorted_values), std::end(sorted_values), 0.0, [&](double acc, double value) {
- return acc + std::pow(value, 2) / gsl::narrow_cast<double>(sorted_values.size());
+ return acc + (std::pow(value, 2) / gsl::narrow_cast<double>(sorted_values.size()));
Review Comment:
once again I disagree with the parens: division has higher precedence than addition, and I don't think anyone finds that confusing. same as in math
##########
.clang-tidy:
##########
@@ -1,6 +1,7 @@
Checks: >
-clang-analyzer-optin.cplusplus.VirtualCall,
-clang-analyzer-optin.core.EnumCastOutOfRange,
+ -clang-analyzer-unix.BlockInCriticalSection,
Review Comment:
why was this disabled?
##########
core-framework/src/utils/SystemCpuUsageTracker.cpp:
##########
@@ -44,7 +44,7 @@ void SystemCpuUsageTracker::queryCpuTimes() {
previous_total_sys_ = total_sys_;
previous_total_idle_ = total_idle_;
gsl::owner<FILE*> file = fopen("/proc/stat", "r");
- if (fscanf(file, "cpu %lu %lu %lu %lu", &total_user_, &total_user_low_, &total_sys_, &total_idle_) != 4) { // NOLINT(cert-err34-c)
+ if (fscanf(file, "cpu %lu %lu %lu %lu", &total_user_, &total_user_low_, &total_sys_, &total_idle_) != 4) { // NOLINT(cert-err34-c,clang-analyzer-unix.Stream)
Review Comment:
This warning looks valid to me. We should handle the NULL case.
```suggestion
if (!file) { return; }
if (fscanf(file, "cpu %lu %lu %lu %lu", &total_user_, &total_user_low_, &total_sys_, &total_idle_) != 4) { // NOLINT(cert-err34-c)
```
##########
extensions/standard-processors/tests/unit/SplitTextTests.cpp:
##########
@@ -81,7 +81,7 @@ TEST_CASE("Test LineReader with input larger than buffer length") {
TEST_CASE("Test LineReader with input of same size as buffer length") {
auto stream = std::make_shared<io::BufferStream>();
- std::string input = std::string(BUFFER_SIZE - 1, 'a') + "\n" + std::string(BUFFER_SIZE * 2 - 1, 'b') + "\n";
+ std::string input = std::string(BUFFER_SIZE - 1, 'a') + "\n" + std::string((BUFFER_SIZE * 2) - 1, 'b') + "\n";
Review Comment:
same thing about parens here
##########
core-framework/src/utils/SystemCpuUsageTracker.cpp:
##########
@@ -76,7 +76,7 @@ double SystemCpuUsageTracker::getCpuUsageBetweenLastTwoQueries() const {
if (total_diff + total_idle_diff == 0) {
return -1.0;
}
- double percent = static_cast<double>(total_diff) / static_cast<double>(total_diff + total_idle_diff);
+ double percent = static_cast<double>(total_diff) / static_cast<double>(total_diff + total_idle_diff); // NOLINT(clang-analyzer-optin.taint.TaintedDiv)
Review Comment:
I'd rather disable this check globally. If it can't prove from the previous check that the denominator is not zero, then it's mostly useless and not worth the overhead of maintaining NOLINT comments.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2629 Clean up minifi-api includes [nifi-minifi-cpp]
Posted by "lordgamez (via GitHub)" <gi...@apache.org>.
lordgamez commented on code in PR #2029:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2029#discussion_r2372156116
##########
core-framework/src/utils/SystemCpuUsageTracker.cpp:
##########
@@ -44,7 +44,7 @@ void SystemCpuUsageTracker::queryCpuTimes() {
previous_total_sys_ = total_sys_;
previous_total_idle_ = total_idle_;
gsl::owner<FILE*> file = fopen("/proc/stat", "r");
- if (fscanf(file, "cpu %lu %lu %lu %lu", &total_user_, &total_user_low_, &total_sys_, &total_idle_) != 4) { // NOLINT(cert-err34-c)
+ if (fscanf(file, "cpu %lu %lu %lu %lu", &total_user_, &total_user_low_, &total_sys_, &total_idle_) != 4) { // NOLINT(cert-err34-c,clang-analyzer-unix.Stream)
Review Comment:
Updated in https://github.com/apache/nifi-minifi-cpp/pull/2029/commits/cf510446553c2545f87c74e4f3f28b46deaa3df4
##########
extensions/sftp/processors/ListSFTP.cpp:
##########
@@ -690,7 +690,7 @@ void ListSFTP::listByTrackingEntities(
time_t now = time(nullptr);
uint64_t min_timestamp_to_list = (!initial_listing_complete_ && entity_tracking_initial_listing_target_ == ENTITY_TRACKING_INITIAL_LISTING_TARGET_ALL_AVAILABLE)
- ? 0U : (now * 1000 - entity_tracking_time_window.count());
+ ? 0U : ((now * 1000) - entity_tracking_time_window.count());
Review Comment:
Removed check and updated in https://github.com/apache/nifi-minifi-cpp/pull/2029/commits/cf510446553c2545f87c74e4f3f28b46deaa3df4
##########
extensions/standard-processors/processors/AttributeRollingWindow.cpp:
##########
@@ -113,7 +113,7 @@ void AttributeRollingWindow::calculateAndSetAttributes(core::FlowFile &flow_file
}());
// https://math.stackexchange.com/questions/1720876/sums-of-squares-minus-square-of-sums
const auto avg_of_squares = std::accumulate(std::begin(sorted_values), std::end(sorted_values), 0.0, [&](double acc, double value) {
- return acc + std::pow(value, 2) / gsl::narrow_cast<double>(sorted_values.size());
+ return acc + (std::pow(value, 2) / gsl::narrow_cast<double>(sorted_values.size()));
Review Comment:
Removed check and updated in https://github.com/apache/nifi-minifi-cpp/pull/2029/commits/cf510446553c2545f87c74e4f3f28b46deaa3df4
##########
extensions/standard-processors/processors/SplitText.cpp:
##########
@@ -53,7 +53,7 @@ uint8_t LineReader::getEndLineSize(size_t newline_position) {
}
void LineReader::setLastLineInfoAttributes(uint8_t endline_size, const std::optional<std::string>& starts_with) {
- const uint64_t size_from_beginning_of_stream = (current_buffer_count_ - 1) * buffer_size_ + buffer_offset_;
+ const uint64_t size_from_beginning_of_stream = ((current_buffer_count_ - 1) * buffer_size_) + buffer_offset_;
Review Comment:
Removed check and updated in https://github.com/apache/nifi-minifi-cpp/pull/2029/commits/cf510446553c2545f87c74e4f3f28b46deaa3df4
##########
extensions/standard-processors/tests/unit/SplitTextTests.cpp:
##########
@@ -81,7 +81,7 @@ TEST_CASE("Test LineReader with input larger than buffer length") {
TEST_CASE("Test LineReader with input of same size as buffer length") {
auto stream = std::make_shared<io::BufferStream>();
- std::string input = std::string(BUFFER_SIZE - 1, 'a') + "\n" + std::string(BUFFER_SIZE * 2 - 1, 'b') + "\n";
+ std::string input = std::string(BUFFER_SIZE - 1, 'a') + "\n" + std::string((BUFFER_SIZE * 2) - 1, 'b') + "\n";
Review Comment:
Removed check and updated in https://github.com/apache/nifi-minifi-cpp/pull/2029/commits/cf510446553c2545f87c74e4f3f28b46deaa3df4
##########
extensions/standard-processors/utils/JoltUtils.h:
##########
@@ -58,7 +58,7 @@ class Spec {
return nullptr;
}
- template<std::invocable<std::shared_ptr<core::logging::Logger>> OnEnterFn, std::invocable<std::shared_ptr<core::logging::Logger>> OnExitFn>
+ template<std::invocable<const std::shared_ptr<core::logging::Logger>&> OnEnterFn, std::invocable<const std::shared_ptr<core::logging::Logger>&> OnExitFn>
Review Comment:
Updated in https://github.com/apache/nifi-minifi-cpp/pull/2029/commits/cf510446553c2545f87c74e4f3f28b46deaa3df4
##########
libminifi/test/unit/ResponseNodeLoaderTests.cpp:
##########
@@ -49,7 +50,7 @@ class ResponseNodeLoaderTestFixture {
}
protected:
- template<typename T, typename = typename std::enable_if_t<std::is_base_of_v<minifi::core::ProcessorImpl, T>>>
+ template<typename T> requires(std::is_base_of_v<minifi::core::ProcessorImpl, T>)
Review Comment:
Removed check and updated in https://github.com/apache/nifi-minifi-cpp/pull/2029/commits/cf510446553c2545f87c74e4f3f28b46deaa3df4
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2629 Clean up minifi-api includes [nifi-minifi-cpp]
Posted by "lordgamez (via GitHub)" <gi...@apache.org>.
lordgamez commented on code in PR #2029:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2029#discussion_r2369375348
##########
core-framework/src/utils/SystemCpuUsageTracker.cpp:
##########
@@ -76,7 +76,7 @@ double SystemCpuUsageTracker::getCpuUsageBetweenLastTwoQueries() const {
if (total_diff + total_idle_diff == 0) {
return -1.0;
}
- double percent = static_cast<double>(total_diff) / static_cast<double>(total_diff + total_idle_diff);
+ double percent = static_cast<double>(total_diff) / static_cast<double>(total_diff + total_idle_diff); // NOLINT(clang-analyzer-optin.taint.TaintedDiv)
Review Comment:
When clang tidy was introduced I remember that it found cases where 0 division was a possible issue, so I wouldn't disable it for 1 false positive if it can help in other cases.
##########
extensions/kafka/PublishKafka.cpp:
##########
@@ -112,7 +112,7 @@ class PublishKafka::Messages {
}
template<typename Func>
- auto modifyResult(size_t index, Func fun) -> decltype(fun(flow_files_.at(index))) {
+ auto modifyResult(size_t index, const Func& fun) -> decltype(fun(flow_files_.at(index))) {
Review Comment:
Clang tidy warns for every unnecessary copy and suggests const ref instead.
##########
.clang-tidy:
##########
@@ -1,6 +1,7 @@
Checks: >
-clang-analyzer-optin.cplusplus.VirtualCall,
-clang-analyzer-optin.core.EnumCastOutOfRange,
+ -clang-analyzer-unix.BlockInCriticalSection,
Review Comment:
As far as I remember there was an issue with a system call in a loop which did make sense in the context, and I think we should be able to have more judgement around these performance issues than the checker, so I didn't find this useful.
##########
extensions/sftp/processors/ListSFTP.cpp:
##########
@@ -690,7 +690,7 @@ void ListSFTP::listByTrackingEntities(
time_t now = time(nullptr);
uint64_t min_timestamp_to_list = (!initial_listing_complete_ && entity_tracking_initial_listing_target_ == ENTITY_TRACKING_INITIAL_LISTING_TARGET_ALL_AVAILABLE)
- ? 0U : (now * 1000 - entity_tracking_time_window.count());
+ ? 0U : ((now * 1000) - entity_tracking_time_window.count());
Review Comment:
We should decide if we want to follow this check at all and explicitly put all parentheses in expressions or just disable this check altogether. I suppose it is better to judge ourselves which expression is complex enough to require explicit parentheses so we can disable this check.
##########
extensions/standard-processors/utils/JoltUtils.h:
##########
@@ -58,7 +58,7 @@ class Spec {
return nullptr;
}
- template<std::invocable<std::shared_ptr<core::logging::Logger>> OnEnterFn, std::invocable<std::shared_ptr<core::logging::Logger>> OnExitFn>
+ template<std::invocable<const std::shared_ptr<core::logging::Logger>&> OnEnterFn, std::invocable<const std::shared_ptr<core::logging::Logger>&> OnExitFn>
Review Comment:
This was again the check that suggests using cont refs instead of copying for the lambda parameters. I think it's better in this case to avoid increasing the shared pointer ref counter when it's unnecessary.
##########
extensions/systemd/ConsumeJournald.cpp:
##########
@@ -78,7 +78,7 @@ void ConsumeJournald::onSchedule(core::ProcessContext& context, core::ProcessSes
return process_old_messages ? journal.seekHead() : journal.seekTail();
};
worker_->enqueue([this, &seek_default] {
- const auto cursor = state_manager_->get() | utils::transform([](std::unordered_map<std::string, std::string>&& m) { return m.at(CURSOR_KEY); });
+ const auto cursor = state_manager_->get() | utils::transform([](const std::unordered_map<std::string, std::string>& m) { return m.at(CURSOR_KEY); });
Review Comment:
clang tidy warns for using rvalue references for parameters that are not moved in the function body
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org