docs: Remove redundant table of contents#27235
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRemoves redundant top-of-page table-of-contents sections from multiple Sphinx .rst documentation pages so the layout relies on the built-in sidebar/inline TOC instead of duplicated content. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
steveburnett
left a comment
There was a problem hiding this comment.
Thanks for all the time you took examining this issue! I don't agree that the majority of the affected pages in this PR would benefit from the table of contents being removed from the top of the page. This is partly because I don't find a clear distinction between the examples of pages to remove the table of contents from, and pages that are recommended should stay unchanged.
For example, this PR recommends removing the table of contents from
https://prestodb.github.io/docs/0.296/connector/kudu.html
but not from
https://prestodb.github.io/docs/0.296/connector/elasticsearch.html
I do agree with the removal recommendation for the following pages in this PR:
https://prestodb.github.io/docs/0.296/cache/local.html
https://prestodb.github.io/docs/0.296/cache/service.html
https://prestodb.github.io/docs/0.296/ecosystem/list.html
https://prestodb.github.io/docs/0.296/router/scheduler.html
Revise this PR to these four pages and I'll approve it. Thanks again!
|
@steveburnett Thank you for the review! The criterion I used to distinguish the pages is: "Does the table of contents on the right fit the screen?" Please let me know your thoughts after another look, and I’ll update the PR accordingly. |
|
Thanks for the explanation! I've considered your reasoning and grouping. I don't consider a redundant table of contents as a bad thing as different readers will use different paths to find the information they want. That said, your grouping helps me understand your goals, and now I am okay with removing the table of contents from these pages: https://prestodb.github.io/docs/0.296/cache/local.html At this time, I'm also okay with changing the level on Hope this helps! |
e2040f4 to
b0eea2b
Compare
steveburnett
left a comment
There was a problem hiding this comment.
LGTM! (docs)
Pull updated branch, new local doc build, looks good. Thanks!
|
Hi @dnskr, could you rebase your branch? I fixed the arrow flight GH action issue on the upstream. After rebase, your doc-only PR should be able to skip the arrowflight CI job. |
b0eea2b to
6966b1f
Compare
Description
Remove redundant table of contents from some documentation pages.
Motivation and Context
The Presto documentation layout features a table of contents on the right side of all pages.
On pages where this sidebar remains visible within the screen view, the identical (or nearly identical) table of contents at the top can be removed as redundant.
Impact
No
Build the documentation and check that the pages are correct:
Contributor checklist
Release Notes
Summary by Sourcery
Documentation: