liurenjie1024 commented on code in PR #1165: URL: https://github.com/apache/iceberg-rust/pull/1165#discussion_r2031166705
########## crates/iceberg/src/puffin/metadata.rs: ########## @@ -91,13 +91,13 @@ impl Flag { /// Metadata about a puffin file. /// For more information, see: https://iceberg.apache.org/puffin-spec/#filemetadata #[derive(Debug, PartialEq, Eq, Serialize, Deserialize, Clone)] -pub(crate) struct FileMetadata { +pub struct FileMetadata { /// Metadata about blobs in file - pub(crate) blobs: Vec<BlobMetadata>, + pub blobs: Vec<BlobMetadata>, Review Comment: Ditto. ########## crates/iceberg/src/puffin/metadata.rs: ########## @@ -26,33 +26,33 @@ use crate::{Error, ErrorKind, Result}; /// Human-readable identification of the application writing the file, along with its version. /// Example: "Trino version 381" -pub(crate) const CREATED_BY_PROPERTY: &str = "created-by"; +pub const CREATED_BY_PROPERTY: &str = "created-by"; /// Metadata about a blob. /// For more information, see: https://iceberg.apache.org/puffin-spec/#blobmetadata #[derive(Debug, PartialEq, Eq, Serialize, Deserialize, Clone)] #[serde(rename_all = "kebab-case")] -pub(crate) struct BlobMetadata { +pub struct BlobMetadata { /// See blob types: https://iceberg.apache.org/puffin-spec/#blob-types - pub(crate) r#type: String, + pub r#type: String, Review Comment: In general we should not expose fields directly in public api, I would suggest to use getter methods instead. ########## crates/iceberg/src/puffin/mod.rs: ########## @@ -18,15 +18,23 @@ //! Iceberg Puffin implementation. #![deny(missing_docs)] -// Temporarily allowing this while crate is under active development -#![allow(dead_code)] mod blob; +pub use blob::{Blob, APACHE_DATASKETCHES_THETA_V1}; + mod compression; +pub use compression::CompressionCodec; + mod metadata; +pub use metadata::{BlobMetadata, FileMetadata, CREATED_BY_PROPERTY}; + #[cfg(feature = "tokio")] mod reader; +#[cfg(feature = "tokio")] +pub use reader::PuffinReader; Review Comment: We could remove `#[cfg(feature = "tokio")]` since puffin reader should be included in default features, but this requires https://github.com/apache/iceberg-rust/pull/1173 to be merged first. -- 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