Skip to content

Conversation

liurenjie1024
Copy link
Contributor

Which issue does this PR close?

What changes are included in this PR?

Fix doc and remove unnecessary ignore attribute in doc.

Are these changes tested?

CI.

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR removes the unnecessary ignore attribute from Rust code examples in the documentation and updates the import path for MemoryCatalog to reference the iceberg crate.

  • Removed ignore attribute from two fenced code blocks
  • Updated use crate::MemoryCatalog; to use iceberg::MemoryCatalog;
Comments suppressed due to low confidence (4)

crates/iceberg/src/writer/mod.rs:41

  • Removing ignore will make these examples compiled and run as doc tests, but they depend on the iceberg-catalog-memory crate which isn’t enabled by default. Consider using rust,no_run (rust,no_run) or gating the examples behind a Cargo feature to avoid CI failures.
//! ```rust

crates/iceberg/src/writer/mod.rs:100

  • This fenced code block was similarly un-ignored. It will now be built as a doc test and fail without the iceberg-catalog-memory feature. Use rust,no_run or revert to ignore, or gate it behind a feature.
//! ```rust

crates/iceberg/src/writer/mod.rs:58

  • The MemoryCatalog type lives in the iceberg_catalog_memory crate, not directly in iceberg. Update this import to use iceberg_catalog_memory::MemoryCatalog or re-export it through the main crate.
//! use iceberg::MemoryCatalog;

crates/iceberg/src/writer/mod.rs:115

  • Same as above: MemoryCatalog isn’t in the iceberg crate root. Change to use iceberg_catalog_memory::MemoryCatalog or ensure it’s re-exported properly.
//! use iceberg::MemoryCatalog;

Xuanwo
Xuanwo previously approved these changes Jul 13, 2025
kevinjqliu
kevinjqliu previously approved these changes Jul 14, 2025
Copy link
Contributor

@kevinjqliu kevinjqliu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

The previous PR made 3 changes to rust, ignore. 2 are captured by this PR already.
The third is here. Should we change this too?

@liurenjie1024
Copy link
Contributor Author

LGTM!

The previous PR made 3 changes to rust, ignore. 2 are captured by this PR already. The third is here. Should we change this too?

Thanks for pointing out, done.

@liurenjie1024 liurenjie1024 merged commit aeff4e5 into apache:main Jul 14, 2025
17 checks passed
jdcasale pushed a commit to jdcasale/iceberg-rust that referenced this pull request Jul 28, 2025
## Which issue does this PR close?

- Closes apache#1503 .

## What changes are included in this PR?

Fix doc and remove  unnecessary `ignore` attribute in doc.

## Are these changes tested?

CI.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove ignore attribute in iceberg crate's doc.
3 participants