fgerlits commented on code in PR #1739:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1739#discussion_r1532461193


##########
libminifi/include/core/json/JsonFlowSerializer.h:
##########
@@ -26,12 +26,12 @@ class JsonFlowSerializer : public 
core::flow::FlowSerializer {
   explicit JsonFlowSerializer(rapidjson::Document document) : 
flow_definition_json_(std::move(document)) {}
 
   [[nodiscard]] std::string serialize(const core::ProcessGroup& process_group, 
const core::flow::FlowSchema& schema, const utils::crypto::EncryptionProvider& 
encryption_provider,
-      const std::unordered_map<utils::Identifier, 
std::unordered_map<std::string, std::string>>& overrides) const override;
+      const core::flow::Overrides& overrides) const override;
 
  private:
   void encryptSensitiveProperties(rapidjson::Value& property_jsons, 
rapidjson::Document::AllocatorType& alloc,
       const std::map<std::string, Property>& properties, const 
utils::crypto::EncryptionProvider& encryption_provider,
-      std::unordered_map<std::string, std::string> component_overrides) const;
+      const core::flow::Overrides& overrides, const utils::Identifier& 
component_id) const;

Review Comment:
   Yes, it is intentional: it seemed simpler to just pass along a (const) 
reference to the `Overrides` object along with the processor/controller service 
ID, so the function can call the `get` functions on the object with the ID.
   
   The other options I considered were to either pass in 
`required_component_overrides` and `optional_component_overrides` separately, 
or create a new type which contains these. I didn't like either of these 
options. Do you have a third idea which is better than the current code?



-- 
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-unsubscr...@nifi.apache.org

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

Reply via email to