Revert "Fix docs for IO readers and strings_convert" - #15872
Conversation
|
Are text docs useful? Can we skip them? |
|
|
||
| .. doxygengroup:: io_readers | ||
| :members: | ||
| :desc-only: |
There was a problem hiding this comment.
Can we add a comment about why these pages are broken? Or a TODO note that readers can see, so the page isn't empty?
There was a problem hiding this comment.
I'd rather not make any additional changes in this PR.
Let's add whatever comments, etc. to a 24.08 PR instead.
|
The problematic tables are almost certainly the same ones that I originally encountered in #13846. I included fixes in 0663d2e, but reverted the fixes in b2dae66 because they broke the vanilla doxygen build. I do not recall finding any format for the tables that successfully built under both vanilla doxygen and Sphinx + Breathe. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## branch-24.06 #15872 +/- ##
===============================================
Coverage ? 83.83%
===============================================
Files ? 137
Lines ? 22987
Branches ? 0
===============================================
Hits ? 19271
Misses ? 3716
Partials ? 0 ☔ View full report in Codecov by Sentry. |
This reverts commit 2b031e0. We got the go ahead to remove the text docs from @taureandyernv. Authors: - Thomas Li (https://github.com/lithomas1) Approvers: - Vyas Ramasubramani (https://github.com/vyasr) URL: #15941
Reverts #15842
The files the original PR added documentation for appear to contain some text that is problematic for the Sphinx parser to extract from doxygen. My best guess is that it's something in a table, since parsing doxygen tables via Breathe is something I know can be tricky. We didn't catch this issue because we currently only build the text docs in nightly builds, not PRs, and this issue only arises in those text builds. We can revisit adding these docs in 24.08. For the sake of correctness, I have added back building text docs in PRs in this PR (see #14856 for context on the removal).