adamdebreceni commented on a change in pull request #926:
URL: https://github.com/apache/nifi-minifi-cpp/pull/926#discussion_r509027132



##########
File path: libminifi/include/utils/Id.h
##########
@@ -65,10 +67,11 @@ class Identifier {
 
   bool operator!=(const Identifier& other) const;
   bool operator==(const Identifier& other) const;
+  bool operator<(const Identifier& other) const;
 
   bool isNil() const;
 
-  std::string to_string() const;
+  SmallString<36> to_string() const;

Review comment:
       previously I wrote:
   
   > I generally like if the types convey their usage, so we can have functions 
expecting `UUIDString`
   
   this would make sense if we wanted functions expecting a `UUIDString`, I 
argue that we don't and wherever we pass/store a stringified `Identifier` we 
instead should pass/store an `Identifier`, 
[this](https://issues.apache.org/jira/browse/MINIFICPP-1395) is the issue 
aiming to make this change, where I think `UUIDString` would be removed
   
   in light of this I would remove the `UUIDString` typedef 




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