-
Notifications
You must be signed in to change notification settings - Fork 915
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
Revert "Fix docs for IO readers and strings_convert" #15872
Conversation
Are text docs useful? Can we skip them? |
@@ -2,4 +2,4 @@ Io Readers | |||
========== | |||
|
|||
.. doxygengroup:: io_readers | |||
:members: | |||
:desc-only: |
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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).