jklamer commented on a change in pull request #1580:
URL: https://github.com/apache/avro/pull/1580#discussion_r820296389



##########
File path: lang/rust/avro/src/schema.rs
##########
@@ -435,15 +474,15 @@ fn parse_json_integer_for_decimal(value: 
&serde_json::Number) -> Result<DecimalM
 
 #[derive(Default)]
 struct Parser {
-    input_schemas: HashMap<String, Value>,
+    input_schemas: HashMap<Name, Value>,

Review comment:
       This makes me happy

##########
File path: lang/rust/avro/src/schema.rs
##########
@@ -292,6 +317,20 @@ impl Name {
     }
 }
 
+impl Hash for Name {

Review comment:
       Is there a reason to not use derive to implement Hash? 
   

##########
File path: lang/rust/avro/src/schema.rs
##########
@@ -241,17 +242,41 @@ impl Name {
         }
     }
 
+    /// Create a new `Name`.
+    /// Parses the optional `namespace` from the `name` string.
+    /// `aliases` will not be defined.
+    pub fn parse_str(name: &str) -> Name {

Review comment:
       I think It would make sense to run this on the input to `Name::new`. I 
think it would make the SDK more consistent. I would be worried by confusing of 
`Name::new(&schema_str[..]) != Name::parse_str(&schema_str[..])`




-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to