-
Notifications
You must be signed in to change notification settings - Fork 162
docs: generate driver status from README badges #2890
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
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 this and sorry I didn't review it earlier!
A few comments,
- The colored badges on the status table are visually very heavy and I think it'd be better to keep it plain text. More vibrant badges could be used on the individual readmes.
- What do you think about breaking the driver status info into its own section and linking the table header to that section?
- I wish the Sphinx badges were structured like
[some badge label: some value]and not just[some value]. The badges look okay but they're harder to interpret without a label (like shields.io badges) and they're also not accessible. Happy to take this on as an improvement.
Edit: Re (3): I see you've mostly used shields.io badges which is great. There are still some remaining sphinx-style badges, can these be changed too?
docs/source/ext/adbc_misc.py
Outdated
| badge_type = row[0].badge_type | ||
| generated_lines.append(f" - :bdg-{badge_type}:`{row[0].status}`") |
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.
| badge_type = row[0].badge_type | |
| generated_lines.append(f" - :bdg-{badge_type}:`{row[0].status}`") | |
| generated_lines.append(f" - {row[0].status}") |
The visual weight this has in the table is relatively high. I think badges on the individual driver README pages is fine but I wonder if we can retain the plain-text formatting of the statuses in this table?
| ================= | ||
|
|
||
| **Available for:** C/C++, GLib/Ruby, Go, Python, R | ||
| .. adbc_driver_status:: ../../../c/driver/postgresql/README.md |
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.
(commenting here because my change is outside the diff)
Should we remove the note below about "The PostgreSQL driver is in beta."? It appears to be out of date and we probably only want to track status in one place.
I thought I changed all of these, where do you see 'sphinx style' badges? (Also what is meant by 'sphinx style'? 😅) |
Do you mean linking the table rows? I do intend to do that, just haven't updated all the doc pages. I'm not sure the status info is "heavy" enough to warrant an entire separate section though. I will update the badges in the Sphinx docs to have a label (so 'Status: Stable' not just 'Stable'). Unfortunately Bootstrap badges don't seem to support the two-tone thing that shields.io does |
|
Hmm. I think I'll split the badges and the installation on the driver page. |
Something like this: patchdiff --git a/docs/source/driver/status.rst b/docs/source/driver/status.rst
index ec05572bd..06bab08ea 100644
--- a/docs/source/driver/status.rst
+++ b/docs/source/driver/status.rst
@@ -28,10 +28,6 @@ Driver Implementation Status
Implementation Status
=====================
-**Experimental** drivers are not feature-complete and the implementation is still progressing.
-**Beta** drivers are (mostly) feature-complete but have only been available for a short time.
-**Stable** drivers are (mostly) feature-complete (as much as possible for the underlying database) and have been available/tested for a while.
-
.. note::
Drivers that support C/C++ can also be used from C#, GLib, Go, Python, R,
@@ -59,6 +55,23 @@ Implementation Status
`DuckDB documentation
<https://duckdb.org/docs/stable/clients/adbc.html>`_ for details.
+
+.. _driver_status:
+
+Driver Status
+=============
+
+The drivers in the above table vary with respect to their maturity and have been categorized according to the following groupings:
+
+**Stable**
+ Mostly feature-complete (as much as possible for the underlying database) and have been available/tested for a while.
+
+**Beta**
+ Mostly feature-complete but have only been available for a short time.
+
+**Experimental**
+ Not feature-complete and the implementation is still progressing.
+
Feature Support
===============
diff --git a/docs/source/ext/adbc_misc.py b/docs/source/ext/adbc_misc.py
index cf6aa3aca..9a98d95ef 100644
--- a/docs/source/ext/adbc_misc.py
+++ b/docs/source/ext/adbc_misc.py
@@ -298,7 +298,7 @@ class DriverStatusTableDirective(SphinxDirective):
"",
" * - Vendor",
" - Implementation",
- " - Driver Status",
+ " - :ref:`driver_status`",
" - Packages [#packages]_",
"",
]and diff --git a/docs/source/ext/adbc_misc.py b/docs/source/ext/adbc_misc.py
index cf6aa3aca..ac4e5cd6d 100644
--- a/docs/source/ext/adbc_misc.py
+++ b/docs/source/ext/adbc_misc.py
@@ -248,7 +248,7 @@ class DriverStatusDirective(SphinxDirective):
generated_lines = [
f":bdg-primary:`Language: {status.implementation}`"
- f":bdg-{status.badge_type}:`Status: {status.status}`"
+ f":bdg-ref-{status.badge_type}:`Status: {status.status} <driver_status>`"
]
parsed = docutils.nodes.Element()
|
Co-authored-by: Bryce Mecum <petridish@gmail.com>
|
I'll merge this week if there's no further comments |
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.
+1, thanks







Fixes #2764.