Add docs quality CI gates and clear the doxygen warning backlog#2217
Open
swahtz wants to merge 5 commits into
Open
Add docs quality CI gates and clear the doxygen warning backlog#2217swahtz wants to merge 5 commits into
swahtz wants to merge 5 commits into
Conversation
Motivated by issue AcademySoftwareFoundation#2161 (docs drift away from code), this change wires in a layered set of gates so the same class of rot fails on the PR that introduces it. CI gates (new files): * ci/validate_docs_references.py runs on every PR via a new validate_docs job. It scans doc/**/*.md and doc/**/*.txt and verifies that every #include <...> inside a fenced code block (Markdown or Doxygen @code) resolves to a real header in this checkout, and that every github.com/AcademySoftwareFoundation/openvdb/blob/<ref>/<path> URL points at a file on disk. Catches the exact pattern from AcademySoftwareFoundation#2161 (e.g. nanovdb/util/CudaDeviceBuffer.h -> nanovdb/cuda/DeviceBuffer.h). * ci/check_doxygen_warnings.py filters the Doxygen warning log against doc/.doxygen_warning_suppress.txt and fails the build on any surviving warning. Replaces Doxygen's all-or-nothing WARN_AS_ERROR with a per-pattern gate that honours a documented backlog without blocking new regressions. Reports suppression hit counts so dead patterns are flagged automatically. * doc/.doxygen_warning_suppress.txt is the explicit triage backlog: four narrowly-scoped, heavily-commented regex patterns (doc/changes.txt historical \link rot, vendored ext/imath/Half.h, a Doxygen recursive variadic template false positive in ex_util/ComputePrimitives.h, and the INLINE_INHERITED_MEMB + unnamed-override-param interaction in openvdb_ax/ast/AST.h). * .github/workflows/docs.yml runs the validator first, then doxygen, then the warning check (which fails the job on any unsuppressed warning), and always uploads the doxygen.warnings.log as an artifact. Drops '**.md' from paths-ignore so doc-only PRs are still gated. Single-source-of-truth fix for the bug in AcademySoftwareFoundation#2161: * doc/nanovdb/HelloWorld.md and four backing example sources (ex_openvdb_to_nanovdb_accessor, ex_read_nanovdb_sphere_accessor, ex_read_nanovdb_sphere_accessor_cuda) now use Doxygen's \snippet mechanism instead of duplicating code into markdown. The Hello World rendered on openvdb.org will always match what is built and tested in CI. The exact namespace drift from AcademySoftwareFoundation#2161 is now structurally impossible to reintroduce. * doc/CMakeLists.txt sets DOXYGEN_EXAMPLE_PATH/EXAMPLE_RECURSIVE so \snippet paths resolve, captures DOXYGEN_WARN_LOGFILE, and adds an OPENVDB_DOXYGEN_WARN_AS_ERROR option for local opt-in to Doxygen's native all-or-nothing gate. Doxygen warning backlog (44 of 45 surviving warnings fixed in this PR): * nanovdb/GridHandle.h - bogus @param verbose removed; 3x @param filename -> @param fileName; @param pool -> @param other. * nanovdb/NanoVDB.h - 3x @param ijk -> @param xyz on Map::apply*Jacobian* methods; fixed @node -> @note typo. * nanovdb/util/PrefixSum.h - added missing @param op documentation. * openvdb/TypeList.h - backtick-wrapped 'std::get<I>(mTuple)' so Doxygen stops parsing <I> as an unterminated HTML italics tag. * openvdb/tools/RayTracer.h - added explicit @brief docs to seven shader-subclass operator() overrides so the inherited @param nml/dir no longer conflict with the override signatures. * openvdb/version.h.in - @file path now matches the actual filename. * openvdb_houdini/ParmFactory.h - 2x \link references qualified with the houdini_utils:: namespace so they resolve from class scope. * doc/ax/doc.txt - deleted orphan </div> with no matching <div>. * doc/nanovdb/SourceTree.md - closed the missing code fence; replaced 'foo@bar:~$' bash prompt (Doxygen parses @bar as a command) with the unlabeled '$' style used elsewhere in doc/nanovdb/. * doc/nanovdb/FAQ.md - corrected link to nanovdb/cuda/DeviceBuffer.h (was util/CudaDeviceBuffer.h, a sibling of AcademySoftwareFoundation#2161). LaTeX -> MathJax migration: * DOXYGEN_USE_MATHJAX YES + MATHJAX_VERSION MathJax_3 in doc/CMakeLists.txt switches formula rendering from latex/dvips/PNG to MathJax. Removes the install_latex step from docs.yml and lets contributors build the docs locally without any TeX install (the conda-forge texlive-core package on osx-arm64 is incomplete and has no working alternative on that channel). AX docs use @f$/@f[ math; output is now browser-rendered, selectable, and crisper. Result: 73 raw Doxygen warnings, 73 suppressed by documented patterns, 0 surviving. The strict gate is live and enforces 'no new warnings' from the next PR onward. Signed-off-by: Jonathan Swartz <jonathan@jswartz.info>
swahtz
requested review from
Idclip,
apradhana,
danrbailey,
jmlait,
kmuseth and
richhones
as code owners
Whittles two more entries off doc/.doxygen_warning_suppress.txt by fixing the underlying warnings instead of suppressing them. doc/changes.txt (26 unresolved-link warnings -> 0): * Historical Release Notes \link references to APIs that were since renamed or removed are delinked to @c <name> (the file's existing inline-code style), or dropped to plain prose where the link text was an English word woven into a sentence. createLevelSetSphere() is requalified tools:: (it only lacked the namespace, matching the working link elsewhere in the file). doc/CMakeLists.txt: * EXCLUDE_SYMBOLS no longer carries a capital-I "*Internal*" glob: it also matched the public tree::InternalNode / nanovdb::InternalNode classes, silently dropping them from the docs and breaking InternalNode::childCount in changes.txt while its RootNode/LeafNode siblings resolved. Both classes are now documented and the link resolves. * OPENVDB_API is predefined empty so Doxygen stops folding the export macro into the name of `enum OPENVDB_API FromBitsTag` in the vendored Half.h. ext/imath/openvdb/math/Half.h (vendored, OpenVDB-adapted Imath fork): * @file half.h -> @file Half.h (the file was renamed during vendoring). * The internal half-float limit/config macros are wrapped in @cond/@endcond: Doxygen auto-links the literal `#define` token on documented active macros, and these are implementation details (the half type lives in the excluded internal namespace). * The file-level description now renders cleanly: its ASCII bit-layout diagrams, formulas and example tables are wrapped in <pre>, and its double-backtick code spans are reduced to single backticks (the double form broke Doxygen 1.14's markdown paragraphing and collapsed the prose). doc/.doxygen_warning_suppress.txt: * Drop the changes.txt and Half.h entries; both are now warning-clean. Remaining backlog: the ComputePrimitives.h recursive-template false positive and the AST.h INLINE_INHERITED_MEMB interaction. Verified against Doxygen 1.14 (the CI version): 0 surviving warnings. Signed-off-by: Jonathan Swartz <jonathan@jswartz.info>
…es.h The cxx14 namespace was a C++11 forward-compat reimplementation of std::index_sequence / std::make_index_sequence. NanoVDB requires C++17 (MINIMUM_CXX_STANDARD 17), so the standard versions in <utility> (already included) are guaranteed present, and openvdb/Types.h already uses them. Switching to std:: deletes the recursive-inheritance template (make_index_sequence<N> : make_index_sequence<N-1, ...>) that Doxygen's class-relation analyser flagged as a "potential recursive class relation". Doxygen documents template entities rather than instantiations, so the recursive base resolves back to the same class and looks like a self-loop; it can't see that the <0, Is...> specialization terminates the recursion. Removing the shim eliminates the warning at the source for every compiler, not just the CI Doxygen, so its suppression entry is dropped too. The only remaining backlog entry is the AST.h INLINE_INHERITED_MEMB interaction. Signed-off-by: Jonathan Swartz <jonathan@jswartz.info>
With the changes.txt, Half.h and ComputePrimitives.h warnings fixed, the per-pattern suppression layer has served its purpose. Switch the CI docs build to Doxygen's native FAIL_ON_WARNINGS so any net-new warning fails the PR directly, and remove the filter scaffolding: * .github/workflows/docs.yml passes -DOPENVDB_DOXYGEN_WARN_AS_ERROR=ON and drops the check_doxygen_warnings step. The warning log is still uploaded as an artifact (FAIL_ON_WARNINGS writes it before exiting non-zero). * ci/check_doxygen_warnings.py and doc/.doxygen_warning_suppress.txt removed. * doc/CMakeLists.txt option comment updated to note CI now uses the gate. NOTE: the docs job stays red until the remaining openvdb_ax/ast/AST.h INLINE_INHERITED_MEMB warning (handled separately) is merged into this branch -- it is the only warning the build still emits. Signed-off-by: Jonathan Swartz <jonathan@jswartz.info>
Signed-off-by: Jonathan Swartz <jonathan@jswartz.info>
Contributor
Author
|
Waiting for #2222 to pass before docs tests in this PR will pass. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2161 — the docs site showed
nanovdb::createNanoGridafter the symbol had moved tonanovdb::tools::. Markdown code in our docs is never compiled or tested, so this kind of rot goes unnoticed.Two CI gates (
.github/workflows/docs.yml):validate_docs_references.py— every#includeandgithub.com/.../blob/<ref>/URL in doc code blocks must resolve to a real file.WARN_AS_ERROR=FAIL_ON_WARNINGS, so any new doc warning fails the PR.Clears the entire Doxygen warning backlog at the source — the 117 warnings fixed directly:
doc/changes.txt— 26 historical dead\links to removed/renamed APIs →@c name(or plain prose);createLevelSetSphere()requalifiedtools::.ext/imath/.../Half.h(vendored Imath) — stale@filefixed, internal macros wrapped in@cond, and the file description made to render (ASCII diagrams →<pre>, double-backticks → single, which broke Doxygen 1.14's markdown).nanovdb/.../ex_util/ComputePrimitives.h—cxx14::make_index_sequenceshim →std::(C++17 baseline), dropping a recursive-inheritance template Doxygen mis-flagged.doc/CMakeLists.txt— removed an over-broad*Internal*EXCLUDE_SYMBOLSglob (it hid the publicInternalNodeclasses) and predefinedOPENVDB_API=.Also:
HelloWorld.mdpulls its example via\snippetfrom the compiledex_*sources (the structural #2161 fix); AX math migrated LaTeX → MathJax (drops the TeX toolchain dependency).