arpadboda commented on a change in pull request #789:
URL: https://github.com/apache/nifi-minifi-cpp/pull/789#discussion_r430369684



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

Review comment:
       Do we need the empty file here? 

##########
File path: generateVersion.sh
##########
@@ -37,8 +37,25 @@ IFS=';' read -r -a extensions_array <<< "$extensions"
 extension_list="${extension_list} } "
 
 cat >"$out_dir/agent_version.h" <<EOF
-#ifndef AGENT_BUILD_H
-#define AGENT_BUILD_H
+/**

Review comment:
       Why do we license an autogenerated content?

##########
File path: libminifi/include/core/Connectable.h
##########
@@ -191,11 +192,11 @@ class Connectable : public CoreComponent {
   std::shared_ptr<logging::Logger> logger_;
 };
 
-}
+}  // namespace core

Review comment:
       Inconsistent comment format, especially by leaving the previous one 
below. 

##########
File path: libminifi/include/c2/ControllerSocketProtocol.h
##########
@@ -79,16 +81,15 @@ class ControllerSocketProtocol : public HeartBeatReporter {
   std::shared_ptr<minifi::io::StreamFactory> stream_factory_;
 
  private:
-
   std::shared_ptr<logging::Logger> logger_;
 };
 
 REGISTER_RESOURCE(ControllerSocketProtocol, "Creates a reporter that can 
handle basic c2 operations for a localized environment through a simple TCP 
socket.");
 
-} /* namesapce c2 */
+}  // namespace c2

Review comment:
       Inconsistent comment format

##########
File path: libminifi/include/c2/PayloadParser.h
##########
@@ -186,10 +185,10 @@ class PayloadParser {
   std::string component_to_get_;
 };
 
-} /* namesapce c2 */
+}  // namespace c2

Review comment:
       Inconsistent comment format

##########
File path: libminifi/include/c2/protocols/RESTProtocol.h
##########
@@ -100,10 +98,10 @@ class RESTProtocol {
   std::map<std::string, C2Payload> nested_payloads_;
 };
 
-} /* namesapce c2 */
+}  // namespace c2

Review comment:
       Inconsistent comment format

##########
File path: libminifi/include/core/ContentRepository.h
##########
@@ -104,12 +106,11 @@ class ContentRepository : public 
StreamManager<minifi::ResourceClaim> {
     if (count != count_map_.end() && count->second > 0) {
       count_map_[str] = count->second - 1;
     } else {
-       count_map_.erase(str);
+  count_map_.erase(str);

Review comment:
       Changing the tab to spaces is good, but please change to the correct 
amount. 

##########
File path: libminifi/include/c2/C2Protocol.h
##########
@@ -102,18 +101,17 @@ class C2Protocol : public core::Connectable {
   }
 
  protected:
-
   std::atomic<bool> running_;
 
   std::shared_ptr<core::controller::ControllerServiceProvider> controller_;
 
   std::shared_ptr<Configure> configuration_;
 };
 
-} /* namesapce c2 */
+}  // namespace c2

Review comment:
       Same inconsistency here. 

##########
File path: libminifi/include/c2/C2Payload.h
##########
@@ -189,10 +191,10 @@ class C2Payload : public state::Update {
   bool is_collapsible_{ true };
 };
 
-} /* namesapce c2 */
+}  // namespace c2

Review comment:
       What's the motivation behind this change?
   This just looks more inconsistent now. 

##########
File path: extensions/standard-processors/processors/ListenSyslog.h
##########
@@ -200,20 +207,20 @@ class ListenSyslog : public core::Processor {
   int64_t _maxBatchSize;
   std::string _messageDelimiter;
   std::string _protocol;
-  int64_t _port;bool _parseMessages;
+  int64_t _port; bool _parseMessages;

Review comment:
       That's true, but nah, in case we touch it, let's make it nice, not just 
"pass linter"!

##########
File path: libminifi/include/c2/HeartBeatReporter.h
##########
@@ -89,18 +90,17 @@ class HeartBeatReporter : public core::Connectable {
   }
 
  protected:
-
   std::shared_ptr<core::controller::ControllerServiceProvider> controller_;
 
   std::shared_ptr<state::StateMonitor> update_sink_;
 
   std::shared_ptr<Configure> configuration_;
 };
 
-} /* namesapce c2 */
+}  // namespace c2

Review comment:
       Inconsistent comment format

##########
File path: libminifi/include/c2/C2Trigger.h
##########
@@ -73,10 +74,10 @@ class C2Trigger : public core::Connectable{
   virtual C2Payload getAction() = 0;
 };
 
-} /* namesapce c2 */
+}  // namespace c2

Review comment:
       Inconsistent comment format

##########
File path: libminifi/include/c2/protocols/RESTProtocol.h
##########
@@ -18,28 +18,29 @@
 #ifndef LIBMINIFI_INCLUDE_C2_PROTOCOLS_RESTPROTOCOL_H_
 #define LIBMINIFI_INCLUDE_C2_PROTOCOLS_RESTPROTOCOL_H_
 
-#include <stdexcept>
+#include <map> // NOLINT
+#include <stdexcept> // NOLINT
 
 #ifdef RAPIDJSON_ASSERT
 #undef RAPIDJSON_ASSERT
 #endif
 #define RAPIDJSON_ASSERT(x) if(!(x)) throw std::logic_error("rapidjson 
exception"); //NOLINT
 
+#include <vector> // NOLINT
+#include <string> // NOLINT

Review comment:
       What kind of linter error would it generate?
   Why do we have the rapidjson assert defined in between std includes?

##########
File path: libminifi/include/controllers/SSLContextService.h
##########
@@ -17,9 +17,11 @@
  */
 #ifndef LIBMINIFI_INCLUDE_CONTROLLERS_SSLCONTEXTSERVICE_H_
 #define LIBMINIFI_INCLUDE_CONTROLLERS_SSLCONTEXTSERVICE_H_
+
+#include <string>

Review comment:
       This should be together with the other std includes (iostream, memory)

##########
File path: libminifi/include/core/ProcessSession.h
##########
@@ -49,12 +51,12 @@ class ProcessSession : public ReferenceContainer {
   /*!
    * Create a new process session
    */
-  ProcessSession(std::shared_ptr<ProcessContext> processContext = nullptr)
+  explicit ProcessSession(std::shared_ptr<ProcessContext> processContext = 
nullptr)
       : process_context_(std::move(processContext)),
         logger_(logging::LoggerFactory<ProcessSession>::getLogger()) {
     logger_->log_trace("ProcessSession created for %s", 
process_context_->getProcessorNode()->getName());
     auto repo = process_context_->getProvenanceRepository();
-    //provenance_report_ = new provenance::ProvenanceReporter(repo, 
process_context_->getProcessorNode()->getName(), 
process_context_->getProcessorNode()->getName());
+    // provenance_report_ = new provenance::ProvenanceReporter(repo, 
process_context_->getProcessorNode()->getName(), 
process_context_->getProcessorNode()->getName());

Review comment:
       Please delete this, the line below does it correctly. 

##########
File path: libminifi/include/core/FlowConfiguration.h
##########
@@ -148,11 +152,10 @@ class FlowConfiguration : public CoreComponent {
     for (auto sl_func : get_static_functions().statics_sl_funcs_) {
       core::ClassLoader::getDefaultClassLoader().registerResource("", sl_func);
     }
-    //get_static_functions().statics_sl_funcs_.clear();
+    // get_static_functions().statics_sl_funcs_.clear();

Review comment:
       This seems like a leftover of something, if we don't need it, let's just 
remove it. 

##########
File path: libminifi/include/c2/triggers/FileUpdateTrigger.h
##########
@@ -111,16 +111,17 @@ class FileUpdateTrigger : public C2Trigger {
   std::string file_;
   std::atomic<uint64_t> last_update_;
   std::atomic<bool> update_;
+
  private:
   std::shared_ptr<logging::Logger> logger_;
 };
 // add the trigger to the known resources.
 REGISTER_RESOURCE(FileUpdateTrigger, "Defines a file update trigger when the 
last write time of a file has been changed.")
 
-} /* namesapce c2 */
+}  // namespace c2

Review comment:
       Inconsistent comment format

##########
File path: libminifi/include/core/state/Value.h
##########
@@ -501,11 +496,11 @@ struct SerializedResponseNode {
   SerializedResponseNode &operator=(const SerializedResponseNode &other) = 
default;
 };
 
-} /* namespace metrics */
+}  // namespace response

Review comment:
       This is inconsistent as well

##########
File path: libminifi/include/sitetosite/RawSocketProtocol.h
##########
@@ -52,21 +57,20 @@ namespace sitetosite {
  */
 typedef struct Site2SitePeerStatus {
   std::string host_;
-  int port_;bool isSecure_;
+  int port_; bool isSecure_;

Review comment:
       Please split these!

##########
File path: libminifi/include/processors/ProcessorUtils.h
##########
@@ -1,9 +1,28 @@
-#include <string>
-#include <memory>
-#include <vector>
+/**

Review comment:
       👍 

##########
File path: libminifi/include/core/Processor.h
##########
@@ -312,11 +308,11 @@ class Processor : public Connectable, public 
ConfigurableComponent, public std::
   std::shared_ptr<logging::Logger> logger_;
 };
 
-}
+}  // namespace core

Review comment:
       Inconsistent comment format

##########
File path: libminifi/include/sitetosite/SiteToSite.h
##########
@@ -211,14 +215,14 @@ typedef enum {
 // Respond Code Class
 typedef struct {
   RespondCode code;
-  const char *description;bool hasDescription;
+  const char *description; bool hasDescription;

Review comment:
       Please split these!




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

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to