-
Notifications
You must be signed in to change notification settings - Fork 1.8k
docs: fix broken SQL & DataFrame links in root README (#18153) #18274
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
Conversation
README.md
Outdated
|
|
||
| "Out of the box," | ||
| DataFusion offers [SQL] and [`Dataframe`] APIs, excellent [performance], | ||
| DataFusion offers [SQL](docs/source/user-guide/sql) and [Dataframe](docs/source/user-guide/dataframe.md) APIs, excellent [performance], |
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 line was copied from lib.rs (original PR: #12418).
datafusion/datafusion/core/src/lib.rs
Lines 45 to 46 in 987f333
| //! "Out of the box," DataFusion offers [SQL] and [`Dataframe`] APIs, | |
| //! excellent [performance], built-in support for CSV, Parquet, JSON, and Avro, |
But they were missing the references:
datafusion/datafusion/core/src/lib.rs
Lines 60 to 61 in 987f333
| //! [SQL]: https://datafusion.apache.org/user-guide/sql/index.html | |
| //! [`DataFrame`]: dataframe::DataFrame |
I think we should keep the original one and add references for them.
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.
I think especially for SQL it should link to a page instead of a directory.
For DataFrame I'm fine either way
Jefffrey
left a comment
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 picking this up
README.md
Outdated
|
|
||
| "Out of the box," | ||
| DataFusion offers [SQL] and [`Dataframe`] APIs, excellent [performance], | ||
| DataFusion offers [SQL](docs/source/user-guide/sql) and [Dataframe](docs/source/user-guide/dataframe.md) APIs, excellent [performance], |
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.
I think especially for SQL it should link to a page instead of a directory.
For DataFrame I'm fine either way
Updated links in the README to point to the correct URLs.
|
I have updated the PR according to the code review. Please let me know if further changes are required, thanks! :) |
|
Thanks 🎉 |
…pache#18274) ## Which issue does this PR close? - Closes apache#18153 ## Rationale for this change The SQL and DataFrame documentation links in the root README were broken and returned 404 errors. This PR updates the URLs to point to the correct documentation paths, improving navigation for new users and contributors. ## Are these changes tested? Verified that the new links are valid and accessible.
Which issue does this PR close?
Rationale for this change
The SQL and DataFrame documentation links in the root README were broken and returned 404 errors.
This PR updates the URLs to point to the correct documentation paths, improving navigation for new users and contributors.
Are these changes tested?
Verified that the new links are valid and accessible.