-
Notifications
You must be signed in to change notification settings - Fork 1
Navigation: Dissolve cross-tree references #362
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
Discovered per `ag "\.\.\/"`.
WalkthroughUpdated documentation navigation and content: Admin index removed the performance toctree and revised captions; Start index toctree entries adjusted and “going-further” added; new pages for ingest and search were added; query index retargets search link; modelling/fulltext link style switched to reStructuredText cross-references. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User (Docs site)
participant R as Router
participant P as Page Renderer
rect rgba(100,150,255,0.08)
U->>R: Request "start/ingest" or "start/query/search"
R->>P: Resolve route -> new page (`ingest` / `search`)
P-->>U: Serve new ingest/search content
end
rect rgba(200,230,150,0.08)
U->>R: Request "admin/index"
R->>P: Resolve updated admin index (performance toctree removed)
P-->>U: Serve updated admin index
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
docs/start/index.md
Outdated
Ingesting data <../ingest/index> | ||
ingest | ||
application/index | ||
going-further |
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.
It was intentional that this was below "First Steps". It should perhaps just be moved into "First Steps" as it's basically an overview of the remaining TOC + some. Having it in the end seems very odd as that's after half the references in there.
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.
Something more needs done, but for this iteration I prefer to keep it as it was.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
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.
It was intentional that this was below "First Steps".
You are just referring to the going-further
item? If so, fixed per afe1325.
It should perhaps just be moved into "First Steps" as it's basically an overview of the remaining TOC + some. Having it in the end seems very odd as that's after half the references in there.
@coderabbitai: Please carry this over into a dedicated ticket to be discussed with @bmunkholm. Thanks.
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.
- Start/Index: Refactor "Going further" to "Learn more" on index page #368 will resolve it already.
docs/start/ingest.md
Outdated
(start-ingest)= | ||
# Ingesting data | ||
|
||
:::{card} All data ingestion methods for CrateDB at a glance |
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.
Should this card be moved to the bottom to basically act as "Next Step"?
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.
Sure. Fixed with 1123249.
docs/start/query/search.md
Outdated
(start-search)= | ||
# Search | ||
|
||
:::{card} All search features of CrateDB at a glance |
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.
Move card down at end?
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.
Sure. Fixed with 1123249.
modelling/index | ||
query/index | ||
Ingesting data <../ingest/index> | ||
ingest |
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.
@kneth was also complaining about this navigational hiccup today. 🍀
About
References pointing across the document tree have been reported to display confusing navigation behaviour.
Details
Discovered per
ag "\.\.\/"
.Preview