[GitHub] [avro] jklamer commented on a diff in pull request #1905: AVRO-3633: [Rust] Additional attributes for 'avro_derive' crate

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

jklamer commented on code in PR #1905:
URL: https://github.com/apache/avro/pull/1905#discussion_r991777381


##########
lang/rust/avro_derive/src/lib.rs:
##########
@@ -117,11 +121,20 @@ fn get_data_struct_schema_def(
     let mut record_field_exprs = vec![];
     match s.fields {
         syn::Fields::Named(ref a) => {
-            for (position, field) in a.named.iter().enumerate() {
-                let name = field.ident.as_ref().unwrap().to_string(); // we know everything has a name
+            let mut index: usize = 0;
+            for field in a.named.iter() {
+                let mut name = field.ident.as_ref().unwrap().to_string(); // we know everything has a name
                 let field_attrs =
                     FieldOptions::from_attributes(&field.attrs[..]).map_err(darling_to_syn)?;
                 let doc = preserve_optional(field_attrs.doc);
+                let rename = field_attrs.rename;
+                if rename.is_some() {

Review Comment:
   Probably best to use `if let` syntax here as in:
   ```
   if let Some(rename) = rename {
      name = rename.clone()
   }
   ```



##########
lang/rust/avro_derive/src/lib.rs:
##########
@@ -117,11 +121,20 @@ fn get_data_struct_schema_def(
     let mut record_field_exprs = vec![];
     match s.fields {
         syn::Fields::Named(ref a) => {
-            for (position, field) in a.named.iter().enumerate() {
-                let name = field.ident.as_ref().unwrap().to_string(); // we know everything has a name
+            let mut index: usize = 0;
+            for field in a.named.iter() {
+                let mut name = field.ident.as_ref().unwrap().to_string(); // we know everything has a name
                 let field_attrs =
                     FieldOptions::from_attributes(&field.attrs[..]).map_err(darling_to_syn)?;
                 let doc = preserve_optional(field_attrs.doc);
+                let rename = field_attrs.rename;
+                if rename.is_some() {
+                    name = rename.unwrap();
+                }
+                let skip = field_attrs.skip;
+                if skip.is_some() && skip.unwrap() {

Review Comment:
   Same here
   ```
   if let Some(true) = skip {
   continue;
   }
   ```
   ^^ pretty sure that works. 



##########
lang/rust/avro_derive/tests/derive.rs:
##########
@@ -1362,4 +1362,144 @@ mod test_derive {
             myenum: MyEnum::Bar,
         });
     }
+
+    #[test]
+    fn test_basic_struct_with_skip_attribute() {

Review Comment:
   nice testing



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

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