-
Notifications
You must be signed in to change notification settings - Fork 0
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 doc build on catalog API #5
Conversation
@@ -52,7 +52,7 @@ use std::sync::Arc; | |||
/// ``` | |||
/// # use datafusion_catalog::Session; | |||
/// # use datafusion_common::{Result, exec_datafusion_err}; | |||
/// struct SessionState {} | |||
/// # struct SessionState {} |
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.
Putting a #
in front of this means the statement is not shown in the resulting documentation
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.
thanks for the explanation! ❤️
@@ -63,7 +63,7 @@ use std::sync::Arc; | |||
/// ``` | |||
/// | |||
/// [`SessionState`]: https://docs.rs/datafusion/latest/datafusion/execution/session_state/struct.SessionState.html | |||
/// [TableProvider]: crate::TableProvider | |||
/// [`TableProvider`]: crate::TableProvider |
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 is the fix for the doc failure 🤦
cc @findepi |
thank you! |
This targets the catalog API PR in DataFusion, apache#11516
I broke the doc tests in #4 due a broken link
This PR fixes that (and improves the example slightly)