Skip to content

Conversation

@alamb
Copy link
Contributor

@alamb alamb commented Jul 30, 2024

Which issue does this PR close?

Closes #11728

Rationale for this change

Warnings in cargo doc sometimes mask real bugs as well as making it hard to track down cargo doc errors locally in my iDE

See also #11728

What changes are included in this PR?

  1. Update CI so broken links are treated as errors
  2. Fix all current warnings
  3. Make 2 structures pub so they can be used by users (caught by doc warnings)

Are these changes tested?

Are there any user-facing changes?

@github-actions github-actions bot added sql SQL Planner development-process Related to development process of DataFusion optimizer Optimizer rules core Core DataFusion crate labels Jul 30, 2024
- name: Run cargo doc
run: |
export RUSTDOCFLAGS="-D warnings -A rustdoc::private-intra-doc-links"
export RUSTDOCFLAGS="-D warnings"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this makes all doc warnings an error and does not allow links to private items

Copy link
Member

Choose a reason for hiding this comment

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

RUSTDOCFLAGS in ci/scripts/rust_docs.sh should be updated as well
i am doing this in #11890

pub use arrow_file::ArrowExec;
pub use avro::AvroExec;
pub use csv::{CsvConfig, CsvExec, CsvOpener};
pub use csv::{CsvConfig, CsvExec, CsvExecBuilder, CsvOpener};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this struct, added by @connec recently, actually wasn't publically exported which was flagged by one of the doc tests

///
/// * Step 1: [`ParquetExec::execute`] is called, returning a [`FileStream`]
/// configured to open parquet files with a [`ParquetOpener`].
/// configured to open parquet files with a `ParquetOpener`.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

since ParquetOpener isn't public let's not try to make a link to it

use self::dialect::{DefaultDialect, Dialect};
pub mod dialect;

pub use expr::Unparsed;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unparsed was also not pub, though maybe it does't need to be

Copy link
Member

@jonahgao jonahgao left a comment

Choose a reason for hiding this comment

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

Looks good to me👍

@alamb
Copy link
Contributor Author

alamb commented Aug 1, 2024

Thanks @jonahgao for the review.

@alamb alamb merged commit a4ac082 into apache:main Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Core DataFusion crate development-process Related to development process of DataFusion optimizer Optimizer rules sql SQL Planner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

doc warnings about private items

3 participants