-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix documentation warnings, make CsvExecBuilder and Unparsed pub #11729
Conversation
@@ -234,7 +234,7 @@ jobs: | |||
rust-version: stable | |||
- name: Run cargo doc | |||
run: | | |||
export RUSTDOCFLAGS="-D warnings -A rustdoc::private-intra-doc-links" | |||
export RUSTDOCFLAGS="-D warnings" |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
@@ -35,7 +35,7 @@ pub use self::parquet::{ParquetExec, ParquetFileMetrics, ParquetFileReaderFactor | |||
|
|||
pub use arrow_file::ArrowExec; | |||
pub use avro::AvroExec; | |||
pub use csv::{CsvConfig, CsvExec, CsvOpener}; | |||
pub use csv::{CsvConfig, CsvExec, CsvExecBuilder, CsvOpener}; |
There was a problem hiding this comment.
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
@@ -191,9 +191,9 @@ pub use writer::plan_to_parquet; | |||
/// # Execution Overview | |||
/// | |||
/// * 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`. |
There was a problem hiding this comment.
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
@@ -29,6 +29,8 @@ pub use plan::plan_to_sql; | |||
use self::dialect::{DefaultDialect, Dialect}; | |||
pub mod dialect; | |||
|
|||
pub use expr::Unparsed; |
There was a problem hiding this comment.
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
There was a problem hiding this 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👍
Thanks @jonahgao for the review. |
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 downcargo doc
errors locally in my iDESee also #11728
What changes are included in this PR?
pub
so they can be used by users (caught by doc warnings)Are these changes tested?
Are there any user-facing changes?