JosephLenton commented on code in PR #2937:
URL: https://github.com/apache/iceberg-rust/pull/2937#discussion_r3706070646


##########
crates/iceberg/src/transaction/update_schema.rs:
##########
@@ -32,12 +32,11 @@ use crate::{Error, ErrorKind, Result, TableRequirement, 
TableUpdate};
 // Default ID for a new column. This will be re-assigned to a fresh ID at 
commit time.
 const DEFAULT_FIELD_ID: i32 = 0;
 
-/// Declarative specification for adding a column in [`UpdateSchemaAction`].
+/// Declarative specification for adding a column in an `UpdateSchemaAction`.
 ///
 /// Use helper constructors such as [`AddColumn::optional`] and 
[`AddColumn::required`],
-/// optionally combined with [`AddColumn::with_parent`] and 
[`AddColumn::with_doc`], then pass
-/// the value to
-/// [`UpdateSchemaAction::add_column`].
+/// optionally combined with the builder's `parent` and `doc` setters via
+/// [`AddColumn::builder`], then pass the value to 
`UpdateSchemaAction::add_column`.

Review Comment:
   I can see on the docs there are many types made, which aren't publicly 
shown: 
https://docs.rs/iceberg/latest/iceberg/transaction/struct.Transaction.html
   
   My own opinion is everything named a public API should also be public. I'll 
make `FastAppendAction`, `ExpireSnapshotsAction`, `ReplaceSortOrderAction`, 
`UpdateLocationAction`, `UpdatePropertiesAction`, `UpdateSchemaAction`, 
`UpdateStatisticsAction`, and `UpgradeFormatVersionAction`, all public as well 
(these are all public items referenced from the `Transaction` object).
   
   Their constructors are a mix of public and crate constructors. For now 
@dannycjones should I make them all `pub(crate)`? This would be the smallest 
change to the API (given they weren't public before).



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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to