liurenjie1024 commented on code in PR #1362:
URL: https://github.com/apache/iceberg-rust/pull/1362#discussion_r2142255435


##########
crates/iceberg/src/spec/table_metadata.rs:
##########
@@ -101,7 +101,10 @@ pub const RESERVED_PROPERTIES: [&str; 9] = [
 /// Reference to [`TableMetadata`].
 pub type TableMetadataRef = Arc<TableMetadata>;
 
-#[derive(Debug, PartialEq, Deserialize, Eq, Clone)]
+#[derive(Debug, PartialEq, Deserialize, Eq, Clone, 
typed_builder::TypedBuilder)]
+#[builder(builder_method(name=declarative_builder))]
+#[builder(builder_type(name=TableMetadataDeclarativeBuilder, doc="Build a new 
[`TableMetadata`] in a declarative way. For imperative operations (e.g. 
`add_snapshot`) and creating new TableMetadata, use [`TableMetadataBuilder`] 
instead."))]

Review Comment:
   To be honest, I feel the `TypedBuilder` is a little hard to maintain in such 
a large struct, do you mind to create a standalone struct for this?



##########
crates/iceberg/src/spec/table_metadata.rs:
##########
@@ -101,7 +101,10 @@ pub const RESERVED_PROPERTIES: [&str; 9] = [
 /// Reference to [`TableMetadata`].
 pub type TableMetadataRef = Arc<TableMetadata>;
 
-#[derive(Debug, PartialEq, Deserialize, Eq, Clone)]
+#[derive(Debug, PartialEq, Deserialize, Eq, Clone, 
typed_builder::TypedBuilder)]
+#[builder(builder_method(name=declarative_builder))]
+#[builder(builder_type(name=TableMetadataDeclarativeBuilder, doc="Build a new 
[`TableMetadata`] in a declarative way. For imperative operations (e.g. 
`add_snapshot`) and creating new TableMetadata, use [`TableMetadataBuilder`] 
instead."))]

Review Comment:
   I agree with @findepi about the naming. To me this is more like a 
constructor, but since I'm not a native English speaker, I don't have a good 
suggestion to the name. But it would be lovely if we have a better name.



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to