[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #789: MINIFICPP-1203 - Enable header linting in include directories and resolve linter recommendations

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

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



##########
File path: extensions/standard-processors/processors/GenerateFlowFile.h
##########
@@ -17,8 +17,16 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-#ifndef __GENERATE_FLOW_FILE_H__
-#define __GENERATE_FLOW_FILE_H__
+#ifndef EXTENSIONS_STANDARD_PROCESSORS_PROCESSORS_GENERATEFLOWFILE_H_
+#define EXTENSIONS_STANDARD_PROCESSORS_PROCESSORS_GENERATEFLOWFILE_H_
+
+#include <memory>
+
+#include <string>
+
+#include <utility>
+
+#include <vector>

Review comment:
       Yes, I did not consider this when writing the script adding missing headers. Will write a new one that corrects for these cases.

##########
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:
       I agree, but this is not related to linter recommendations.

##########
File path: extensions/standard-processors/processors/GenerateFlowFile.h
##########
@@ -17,8 +17,16 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-#ifndef __GENERATE_FLOW_FILE_H__
-#define __GENERATE_FLOW_FILE_H__
+#ifndef EXTENSIONS_STANDARD_PROCESSORS_PROCESSORS_GENERATEFLOWFILE_H_
+#define EXTENSIONS_STANDARD_PROCESSORS_PROCESSORS_GENERATEFLOWFILE_H_
+
+#include <memory>
+
+#include <string>
+
+#include <utility>
+
+#include <vector>

Review comment:
       Added a commit fixing these kinds of errors.

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

Review comment:
       It should be shown as deleted :S
   <img width="1552" alt="Screenshot 2020-05-26 at 16 01 23" src="https://user-images.githubusercontent.com/64011968/82909891-28ea1280-9f6a-11ea-87b6-a6ffc0f24c85.png">
   

##########
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:
       So that it is linter-correct :D

##########
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:
       The motivation is that the linter reports on `missing // namespace ...` comments. I wrote a script that replaced all those reported missing/incorrect namespace comments with a proper one (find the script for each change in the commit message), but the other lines did not get reported. As I don't necessarily think that the other format is "less correct", I decided that it was not worth the effort to correct those leftover `/* namespace ... */`-s, not reported by the linter

##########
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:
       See comment above.

##########
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:
       See commment above.

##########
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:
       See commment above.

##########
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:
       Updated.

##########
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:
       I don't know, but it fails on windows otherwise.

##########
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:
       I don't know, but it fails on windows otherwise. This order is the result of careful threading and experimentation. As I don't enjoy working on Windows specific stuffs, I leave the joy of figuring this out to others :D

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

Review comment:
       Resolved by adding reaction to comment.

##########
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:
       Moved there.

##########
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:
       Removed.

##########
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:
       Removed.

##########
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:
       They are now split.

##########
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:
       They are now split.

##########
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:
       Updated.

##########
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:
       They are now split.




----------------------------------------------------------------
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:
users@infra.apache.org