[GitHub] [nifi-minifi-cpp] lordgamez commented on a change in pull request #1225: MINIFICPP-1688: When storing time durations we should use std::chrono…

[GitBox <gi...@apache.org>]

lordgamez commented on a change in pull request #1225:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1225#discussion_r767574536



##########
File path: extensions/sftp/processors/ListSFTP.cpp
##########
@@ -870,7 +871,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)

Review comment:
       Maybe we can write a separate Jira ticket to replace uint64_t timestamp representations like this to std::chrono in this file.

##########
File path: extensions/standard-processors/processors/GetFile.h
##########
@@ -114,7 +114,7 @@ class GetFile : public core::Processor, public state::response::MetricsNodeSourc
   explicit GetFile(const std::string& name, const utils::Identifier& uuid = {})
       : Processor(name, uuid),
         metrics_(std::make_shared<GetFileMetrics>()),
-        last_listing_time_(0) {
+        last_listing_time_() {

Review comment:
       I think this can be removed

##########
File path: extensions/librdkafka/ConsumeKafka.cpp
##########
@@ -25,6 +25,8 @@
 #include "utils/ProcessorConfigUtils.h"
 #include "utils/gsl.h"
 
+using namespace std::chrono_literals;  // NOLINT(build/namespaces)

Review comment:
       In the main CPPLINT.cfg we have `build/namespaces_literals` defined as part of the filter, which should allow chrono_literals withouth the need to use NOLINT. Unfortunately the implementation of this filter in cpplint.py does not seem to allow `std::chrono_literals`, but it does allow `std::literals::chrono_literals`. So we could replace all usages of `using namespace std::chrono_literals` with `using namespace std::literals::chrono_literals` and we can remove the NOLINT comments from those cases.

##########
File path: libminifi/include/sitetosite/RawSocketProtocol.h
##########
@@ -75,9 +75,9 @@ class RawSiteToSiteClient : public sitetosite::SiteToSiteClient {
     peer_ = std::move(peer);
     _batchSize = 0;
     _batchCount = 0;
-    _batchDuration = 0;
-    _batchSendNanos = 5000000000;  // 5 seconds
-    _timeOut = 30000;  // 30 seconds
+    _batchDuration = std::chrono::seconds(0);
+    _batchSendNanos = std::chrono::seconds(5);
+    _timeOut = std::chrono::seconds(30);  // 30 seconds

Review comment:
       The comment is redundant now, it can be removed.

##########
File path: extensions/standard-processors/tests/CPPLINT.cfg
##########
@@ -0,0 +1 @@
+filter=-build/namespaces

Review comment:
       As per my previous comment maybe we could go with `build/namespaces_literals` here as well.

##########
File path: libminifi/src/core/FlowFile.cpp
##########
@@ -40,17 +40,17 @@ FlowFile::FlowFile()
     : CoreComponent("FlowFile"),
       stored(false),
       marked_delete_(false),
-      entry_date_(0),
-      event_time_(0),
-      lineage_start_date_(0),
+      entry_date_(),
+      event_time_(),
+      lineage_start_date_(),

Review comment:
       I suppose these can be removed as well




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