-
Notifications
You must be signed in to change notification settings - Fork 1
Index and Topics: Improve guidance #369
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
WalkthroughDocumentation updates convert multiple topic and homepage sections to card-based layouts, add muted intro blocks and branded tool/framework cards, adjust headings and copy across several pages, and make presentation-only formatting changes without API or export modifications. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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 |
:link: plotly | ||
:link-type: ref | ||
```{div} .float-right | ||
{height=40px loading=lazy} |
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 we migrate these assets away form crate-client-tools? It's not there anymore, so I assume it's a redirect that saves this.
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.
@coderabbitai: Please create a dedicated issue about it. 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.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all references to crate-client-tools in the repository
echo "=== Searching for crate-client-tools references ==="
rg -n "crate-client-tools" --type md -C 2
echo ""
echo "=== Counting total occurrences ==="
rg "crate-client-tools" --type md -c
Length of output: 87
🏁 Script executed:
#!/bin/bash
# Try different search patterns
echo "=== Searching for clients-tools ==="
rg -n "clients-tools" -C 2
echo ""
echo "=== Searching in docs/topic directory ==="
rg -n "github.com/crate/crate-" docs/topic/ -C 1
echo ""
echo "=== Checking the specific file ==="
cat docs/topic/framework/index.md | grep -n "github.com"
Length of output: 14871
🏁 Script executed:
#!/bin/bash
# Create a GitHub issue for migrating assets from crate-clients-tools
gh issue create \
--title "Migrate asset references from crate-clients-tools repository" \
--body "## Summary
The documentation currently references assets (logos, screenshots, diagrams) from the \`crate-clients-tools\` repository, which appears to no longer exist. While these references currently work (likely through redirects), we should migrate these assets to a more stable location.
## Context
This issue was raised during review of PR #369:
- PR: https://github.com/crate/cratedb-guide/pull/369
- Comment: https://github.com/crate/cratedb-guide/pull/369#discussion_r2405278279
- Raised by: @bmunkholm
## Affected Files
The following files contain references to \`github.com/crate/crate-clients-tools/assets/*\`:
### Topic Documentation (3 files, 3 references)
- \`docs/topic/visualize/index.md\` - Plotly logo
- \`docs/topic/framework/index.md\` - Plotly and Streamlit logos (2 refs)
### Integration Documentation (19 files, 37 references)
- \`docs/integrate/hop/index.md\` - Apache Hop screenshots (3 refs)
- \`docs/integrate/mindsdb/index.md\` - MindsDB logo
- \`docs/integrate/prometheus/index.md\` - Prometheus logo and architecture diagram (2 refs)
- \`docs/integrate/telegraf/index.md\` - Telegraf logo
- \`docs/integrate/superset/index.md\` - Dashboard screenshot and Preset Cloud logo (2 refs)
- \`docs/integrate/sql-server/index.md\` - Microsoft and MSSQL logos (2 refs)
- \`docs/integrate/streamlit/index.md\` - Streamlit logo
- \`docs/integrate/pyviz/index.md\` - Datashader examples (2 refs)
- \`docs/integrate/locust/tutorial.md\` - Link reference
- \`docs/integrate/pycaret/index.md\` - PyCaret logo
- \`docs/integrate/node-red/index.md\` - FlowFuse logo
- \`docs/integrate/plotly/index.md\` - Plotly logo and multiple screenshots (7 refs)
- \`docs/integrate/nifi/index.md\` - NiFi screenshots (3 refs)
- \`docs/integrate/mlflow/index.md\` - MLflow logo
- \`docs/integrate/meltano/index.md\` - Meltano logo
- \`docs/integrate/cluvio/index.md\` - Cluvio dashboard screenshots (2 refs)
- \`docs/integrate/datashader/index.md\` - Datashader examples (2 refs)
- \`docs/integrate/dask/index.md\` - Dask logo
- \`docs/connect/cli.md\` - CLI screenshots (3 refs)
- \`docs/connect/mcp/index.md\` - Discussion link
### Other (1 file)
- \`backlog.md\` - Already mentions this migration task
**Total: 23 files with 40+ asset references**
## Recommended Actions
1. Identify a suitable location for these assets (e.g., \`cratedb-guide\` repository itself, or a dedicated assets repository)
2. Download and migrate all referenced assets
3. Update all documentation files with new asset URLs
4. Verify all assets render correctly
5. Consider adding a policy for where to host documentation assets in the future
## Related
- Related to backlog item: \"Migrate all content from \`crate-clients-tools\`\"" \
--assignee amotl \
--repo crate/cratedb-guide
Length of output: 49
@amotl, I've created issue #371 to track the migration of assets from the crate-clients-tools
repository.
The issue documents all 40+ asset references found across 23 files in the documentation, including logos, screenshots, and diagrams. I've organized them by category (topic documentation, integration documentation, etc.) and included recommended actions for the migration.
🔗 #371
🧠 Learnings used
Learnt from: bmunkholm
PR: crate/cratedb-guide#340
File: docs/home/index.md:84-97
Timestamp: 2025-09-25T19:31:54.320Z
Learning: In the CrateDB Guide docs (MyST), the CrateDB Cloud card on the homepage should link to `getting-started` using `:link-type: ref` instead of the previous `cloud:index` intersphinx target. This change was implemented in PR #340 to direct users to the getting started section rather than directly to the Cloud 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.
Wauu - that was a hugely effective assistance from coderabbit.
@mfussenegger you might want to take a look at how CodeRabbit - in this instance - was highly effective.
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 it isn't an optimal example to just create an issue. There have been more valuable notes, e.g. in GH-367.
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.
@amotl Is it the initial overview you refer to?
In the issue example, not only did it create an issue, but went beyond what asked for and found all references.
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.
Well ok, one could also use grep
for that. I am referring to 91a87c9, where the assistant informed about deprecated features/syntax in Docker Compose before.
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.
Ahh, right. That's more influential. The other is just a nice timesaver - I didn't know it could be used like that.
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.
you might want to take a look at how CodeRabbit - in this instance - was highly effective.
This is something grep and sed could do several orders of magnitude cheaper.
I am referring to 91a87c9, where the assistant informed about deprecated features/syntax in Docker Compose before.
scale
doesn't appear deprecated?
https://github.com/compose-spec/compose-spec/blob/main/spec.md#scale
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.
Ah thanks. Maybe it was replicas
that became deprecated and I mixed it up? I think I had a warning from Docker the other day about it. Thanks for revealing the hallucinations, we may want to turn the bot off again or just use it to convert to active voice, where it helped well the other day when bringing in larger amounts of texts.
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.
No blocking comments, just minor suggestions.
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/index.md
(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-09-25T19:31:54.320Z
Learnt from: bmunkholm
PR: crate/cratedb-guide#340
File: docs/home/index.md:84-97
Timestamp: 2025-09-25T19:31:54.320Z
Learning: In the CrateDB Guide docs (MyST), the CrateDB Cloud card on the homepage should link to `getting-started` using `:link-type: ref` instead of the previous `cloud:index` intersphinx target. This change was implemented in PR #340 to direct users to the getting started section rather than directly to the Cloud documentation.
Applied to files:
docs/index.md
📚 Learning: 2025-08-23T04:22:52.597Z
Learnt from: amotl
PR: crate/cratedb-guide#235
File: docs/start/going-further.md:20-38
Timestamp: 2025-08-23T04:22:52.597Z
Learning: In the CrateDB Guide repository, the prefix `crate-reference:` in documentation links (like `crate-reference:index`) is an intersphinx reference that points to external content in the CrateDB Reference Manual documentation. These references are intentionally not pointing to local files within the repository.
Applied to files:
docs/index.md
📚 Learning: 2025-05-18T12:50:36.393Z
Learnt from: amotl
PR: crate/cratedb-guide#204
File: docs/integrate/mcp/community.md:22-33
Timestamp: 2025-05-18T12:50:36.393Z
Learning: In the CrateDB Guide repository, the prefix `ctk:` in documentation links (like `ctk:query/mcp/server`) is an intersphinx reference that points to external content in the CrateDB Toolkit documentation at https://cratedb-toolkit.readthedocs.io/. These references are intentionally not pointing to local files within the repository.
Applied to files:
docs/index.md
⏰ 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)
- GitHub Check: Build docs
0b79bd6
to
be955be
Compare
be955be
to
c53721f
Compare
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.
Actionable comments posted: 1
♻️ Duplicate comments (1)
🧹 Nitpick comments (2)
docs/topic/framework/index.md (2)
14-19
: Prefer card image options over float/clearfix for consistency and responsiveness.Using
.float-right
+.clearfix
can be brittle on narrow viewports. Sphinx‑Design cards support built‑in image slots/options.Example (Django card):
-:::{card} Django -:link: django -:link-type: ref -```{div} .float-right -{height=40px loading=lazy} -``` -Django makes it easier to build better web apps more quickly and with less code. -```{div} .clearfix -``` -::: +:::{card} Django +:link: django +:link-type: ref +:img-url: https://static.djangoproject.com/img/logos/django-logo-positive.svg +:img-alt: Django logo +:img-cls: sd-p-2 +Django makes it easier to build better web apps more quickly and with less code. +:::Apply similarly to other cards to remove float/clearfix usage.
Also applies to: 25-31, 37-43, 48-54, 66-72
75-81
: Grid layout for “Examples” card and width handling.A lone
:width: 50%
card may leave odd whitespace; consider wrapping cards in a grid or using full width.Option A (full width): remove
:width: 50%
.
Option B (grid):::::{grid} 2 :gutter: 2 ::: {grid-item-card} ...existing examples card content... ::: ::: {grid-item-card} Add a second card (e.g., “Resources” or “Tutorials”) to balance the row. ::: ::::
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
docs/handbook/index.md
(3 hunks)docs/index.md
(0 hunks)docs/solution/index.md
(1 hunks)docs/topic/bi/index.md
(1 hunks)docs/topic/framework/index.md
(1 hunks)docs/topic/index.md
(1 hunks)docs/topic/lineage/index.md
(1 hunks)docs/topic/testing/index.md
(1 hunks)docs/topic/timeseries/index.md
(1 hunks)docs/topic/visualize/index.md
(1 hunks)
💤 Files with no reviewable changes (1)
- docs/index.md
✅ Files skipped from review due to trivial changes (1)
- docs/topic/testing/index.md
🚧 Files skipped from review as they are similar to previous changes (5)
- docs/solution/index.md
- docs/topic/visualize/index.md
- docs/topic/bi/index.md
- docs/topic/index.md
- docs/topic/lineage/index.md
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-25T19:31:54.320Z
Learnt from: bmunkholm
PR: crate/cratedb-guide#340
File: docs/home/index.md:84-97
Timestamp: 2025-09-25T19:31:54.320Z
Learning: In the CrateDB Guide docs (MyST), the CrateDB Cloud card on the homepage should link to `getting-started` using `:link-type: ref` instead of the previous `cloud:index` intersphinx target. This change was implemented in PR #340 to direct users to the getting started section rather than directly to the Cloud documentation.
Applied to files:
docs/handbook/index.md
docs/topic/framework/index.md
⏰ 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)
- GitHub Check: Build docs
🔇 Additional comments (8)
docs/topic/framework/index.md (2)
2-10
: Overall structure and copy look good.Title, intro, and consistent alt text/readability are solid. The card approach improves scannability.
12-13
: All internal reference labels verified. Anchors for django, gradio, plotly, pyviz, queryzen, and streamlit exist.docs/topic/timeseries/index.md (1)
4-6
: Intro block looks goodMuted lead-in reads well and uses valid Sphinx‑Design class.
docs/handbook/index.md (5)
6-6
: Title change is clear and consistent“The CrateDB Handbook” improves clarity.
15-26
: Getting Started card: correct ref usageLinking with
:link-type: ref
togetting-started
matches our guideline. Nice.Based on learnings
139-139
: Copy tweak LGTM“Load data from many sources into CrateDB.” reads better.
186-219
: “Solutions and topics” grid reads wellGood use of icons, concise “What’s inside,” and internal refs.
Please confirm the Material icon names render on RTD (e.g.,
lightbulb
,numbers
) and there are no build warnings in the preview.
223-235
: All toctree targets and labels exist Verified presence of all referenced docs and(solutions)
/(topics)
label definitions.
based on Lucene, combines a unique set of features, and is PostgreSQL-compatible. | ||
|
||
 | ||
 |
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.
Add alt text and prefer MyST image directive for control
Current Markdown image lacks alt text and uses a remote URL with query params. Use the image directive with explicit alt/width.
Apply:
-
+```{image} https://cratedb.com/hs-fs/hubfs/nativesql.png
+:alt: CrateDB native SQL illustration
+:width: 800
+```
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
 |
🤖 Prompt for AI Agents
In docs/handbook/index.md around line 131, replace the bare Markdown image that
uses a remote URL with query parameters with a MyST image directive that
provides alt text and explicit width; remove the query params from the URL and
use the {image} directive, adding a :alt: "CrateDB native SQL illustration" and
:width: 800 to the directive so the image is accessible and controlled by the
docs renderer.
About
Rebalance and improve guiding cards on "Index" page and "Topics" sections.
Preview
References