tustvold commented on code in PR #4706:
URL: https://github.com/apache/arrow-rs/pull/4706#discussion_r1296098512


##########
parquet/src/schema/types.rs:
##########
@@ -560,28 +559,30 @@ impl<'a> GroupTypeBuilder<'a> {
     }
 
     /// Sets [`ConvertedType`](crate::basic::ConvertedType) for this field and 
returns itself.
-    pub fn with_converted_type(mut self, converted_type: ConvertedType) -> 
Self {
-        self.converted_type = converted_type;
-        self
+    pub fn with_converted_type(self, converted_type: ConvertedType) -> Self {
+        Self {
+            converted_type,
+            ..self
+        }
     }
 
     /// Sets [`LogicalType`](crate::basic::LogicalType) for this field and 
returns itself.
-    pub fn with_logical_type(mut self, logical_type: Option<LogicalType>) -> 
Self {
-        self.logical_type = logical_type;
-        self
+    pub fn with_logical_type(self, logical_type: Option<LogicalType>) -> Self {
+        Self {
+            logical_type,
+            ..self
+        }
     }
 
     /// Sets a list of fields that should be child nodes of this field.
     /// Returns updated self.
-    pub fn with_fields(mut self, fields: &mut Vec<TypePtr>) -> Self {
-        self.fields.append(fields);
-        self
+    pub fn with_fields(self, fields: Vec<TypePtr>) -> Self {

Review Comment:
   Accepting `&mut Vec<TypePtr>` made for a very unusual interface, changing to 
accept `Vec<TypePtr>` is much "normal"



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