[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #1280: MINIFICPP-1770 Add C2 NO_OPERATION response if property update is NoOp

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

szaszm commented on a change in pull request #1280:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1280#discussion_r831057966



##########
File path: libminifi/include/c2/C2Agent.h
##########
@@ -150,10 +150,16 @@ class C2Agent : public state::UpdateController {
    */
   void handle_describe(const C2ContentResponse &resp);
 
+
+  enum class UpdateResult {
+    NO_UPDATE,
+    UPDATE_SUCCESFUL,

Review comment:
       typo
   ```suggestion
       UPDATE_SUCCESSFUL,
   ```

##########
File path: libminifi/src/Configure.cpp
##########
@@ -30,6 +30,14 @@ namespace nifi {
 namespace minifi {
 
 bool Configure::get(const std::string& key, std::string& value) const {
+  const std::string_view log_prefix = "nifi.log.";

Review comment:
       Could you make this delegate to the `std::optional`-returning overload, to reduce code duplication? And the prefix could be `static constexpr` to force the construction to compile-time.
   ```suggestion
     static constexpr std::string_view log_prefix = "nifi.log.";
   ```




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