-
Notifications
You must be signed in to change notification settings - Fork 1
Connect: Reorganize and clean up section #361
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
WalkthroughReorganizes and expands docs under docs/connect: replaces legacy pages (connect, ide, df/index, orm) with a drivers‑focused index, expands Applications and Python driver pages, adds a natural‑language integration page, and updates general information anchors and heading levels. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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)
⏰ 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 |
https://www.svgrepo.com/svg/508940/ruby The error was: 429 Client Error: Too Many Requests for url: https://www.svgrepo.com/download/508940/ruby.svg
On connect/index: Perhaps we should move "All drivers" card down to the end as it's not about "get started"? That lso fits the location in the TOC. |
In general, I'm not really sold on the concept of the links to the right at the top (and that's about all pages that has this. That could be handled separately, but might as well mention here again). The reason is my attention is drawn to them while reading the left column, not knowing what I'm missing out on and if I should investigate any of that before moving on. I would in this case also remove "obvious" references to wikipedia and assume that's either common knowledge or that people can look it up themselves. Perhaps this belongs more in a general issue... |
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed:
Length of output: 49 @bmunkholm: I've created issue #372 to track this documentation UX concern: #372 The issue captures your feedback about:
You're right that this is a broader concern affecting multiple pages and should be addressed systematically rather than being tied to this specific PR. The issue is assigned to you for further discussion and prioritization. |
Hi Brian. If your concerns are mostly about the primary navigation, this is just a humble patch which already cleans up a few items there to set the stage for subsequent patches. In this spirit, it doesn't mean it is the final solution. Any item can be moved at your or other's disposal, not necessarily connected to this patch. Otherwise, if you have a quick wish for copy-editing on behalf of this very patch, please highlight it by adding an inline comment at the very spot you are referring to. General discussions, also as commandeered to CodeRabbit, could be added to the existing runsheet/tracking ticket instead of creating a separate issue. /cc @coderabbitai |
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
docs/connect/index.md
Outdated
::::{grid-item-card} {material-outlined}`landscape;2em` All drivers | ||
:link: connect-drivers | ||
:link-type: ref | ||
:link-alt: All CrateDB drivers | ||
All available drivers and adapters. | ||
:::: |
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.
@bmunkholm said:
Perhaps we should move "All drivers" card down to the end as it's not about "get started"? That lso fits the location in the TOC.
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 don't know if this will look great. Maybe just remove it? Do you have another idea for the void it would create, to link to a different target?
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.
Agree, it likely won't look too good at the bottom :-(
Maybe we can live without it. It's in the TOC anyway. Yeah, I would vote for removing.
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.
6eff8b4 removes the card.
About
This patch sets the stage for improved guidance within the drivers section, as suggested. It is just a little reorg plus some minor copy-editing. Further improvements as discussed will be submitted using a subsequent patch.
Preview
References