Relax docs warning failures to opt in#1556
Draft
shi-eric wants to merge 1 commit into
Draft
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthrough
ChangesDocs build hardening
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR relaxes Sphinx warning-as-error behavior for local
Confidence Score: 5/5Safe to merge — this is a purely additive opt-in flag with no risk to existing behavior; all CI jobs are correctly updated to pass --warnings-as-errors. The change is well-scoped: local builds become more resilient without any weakening of CI strictness. The env var is correctly set and restored around each sphinx call, the probe has a short timeout with fail-fast semantics, and every CI docs/doctest job (GitHub Actions and GitLab, including the shared .build-docs-common template) was updated. No logic paths were found that could silently swallow errors or produce incorrect output. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["build_docs.py called"] --> B{"--warnings-as-errors\nflag passed?"}
B -- Yes --> C["Set WARP_DOCS_WARNINGS_AS_ERRORS=1\nAdd -W to sphinx args"]
B -- No --> D["Remove WARP_DOCS_WARNINGS_AS_ERRORS\nNo -W flag"]
C --> E["sphinx build_main runs\nconf.py is loaded"]
D --> E
E --> F{"WARP_DOCS_OFFLINE=1?"}
F -- Yes --> G["intersphinx_mapping = {}"]
F -- No --> H{"WARP_DOCS_WARNINGS_AS_ERRORS=1?"}
H -- Yes --> I["intersphinx_mapping = full mapping\n(probe bypassed)"]
H -- No --> J["_inventories_reachable probe\n(2s timeout per host, fail-fast)"]
J -- All reachable --> K["intersphinx_mapping = full mapping"]
J -- Any unreachable --> L["intersphinx_mapping = {}\n(log info message)"]
G --> M["Sphinx build"]
I --> M
K --> M
L --> M
M --> N["Restore previous\nWARP_DOCS_WARNINGS_AS_ERRORS\n(finally block)"]
Reviews (3): Last reviewed commit: "Relax build_docs.py warnings-as-errors t..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/conf.py (1)
421-428: ⚡ Quick winUse Google-style sections in the new function docstring.
The new
_inventories_reachable()docstring is narrative-only; project Python guidelines require Google-style docstrings.Suggested docstring format
def _inventories_reachable(mapping, timeout=5): - """Return True if the network can reach the intersphinx inventory hosts. - - A host that returns *any* HTTP response (even 403/404) is reachable: Sphinx - fetches inventories with its own user-agent and may succeed where a bare - probe is blocked, so only connection-level failures (DNS, refused, timeout) - count as "offline". This avoids wrongly disabling intersphinx in CI just - because a CDN rejects the probe's user-agent. - """ + """Check whether configured intersphinx inventory hosts are reachable. + + Args: + mapping: Intersphinx mapping dictionary. + timeout: Probe timeout in seconds. + + Returns: + ``True`` if configured hosts are reachable. + """As per coding guidelines:
**/*.py: “Use Google-style docstrings.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/conf.py` around lines 421 - 428, The `_inventories_reachable()` docstring in `docs/conf.py` needs to be converted from narrative prose to the project’s Google-style docstring format. Update the function’s docstring to use standard sections such as Args, Returns, and any other applicable headings, while preserving the current behavior description about network reachability and connection-level failures.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/conf.py`:
- Around line 420-442: The _inventories_reachable function returns True
immediately upon encountering the first reachable host (at lines 436 and 439),
without checking if all configured inventory hosts in the mapping are reachable.
This causes the function to enable intersphinx even when some inventories are
unreachable. Remove the early return True statements within the loop and instead
continue iterating through all hosts in mapping.values() to check every
configured inventory. Only return True after confirming that all hosts are
reachable with HTTP responses, and return False only if any host has a
connection-level failure (URL error, timeout, etc.).
---
Nitpick comments:
In `@docs/conf.py`:
- Around line 421-428: The `_inventories_reachable()` docstring in
`docs/conf.py` needs to be converted from narrative prose to the project’s
Google-style docstring format. Update the function’s docstring to use standard
sections such as Args, Returns, and any other applicable headings, while
preserving the current behavior description about network reachability and
connection-level failures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Enterprise
Run ID: 6d279c32-22ab-493c-b64c-881e87e22cfd
📒 Files selected for processing (7)
.github/workflows/ci.yml.github/workflows/sphinx.yml.gitlab-ci.ymlCHANGELOG.mdbuild_docs.pydocs/conf.pydocs/user_guide/contribution_guide.rst
shi-eric
force-pushed
the
ershi/docs-opt-in-strict-build
branch
from
7a11795 to
5d82ae5
Compare
Building docs hardcoded sphinx-build -W, so an unreachable intersphinx inventory (offline machine, sandbox with no egress, flaky DNS) became a fatal error and aborted the build with no output, even though the local content was fine. Make warnings-as-errors opt-in: build_docs.py is lenient by default and takes a new --warnings-as-errors flag, which CI passes to keep strict checks. Also skip external intersphinx resolution in conf.py when the inventories are unreachable or WARP_DOCS_OFFLINE=1 is set, so offline builds still produce output. Signed-off-by: Eric Shi <ershi@nvidia.com>
shi-eric
force-pushed
the
ershi/docs-opt-in-strict-build
branch
from
5d82ae5 to
1cbdc81
Compare
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.
Description
Make docs warning failures opt-in for local
build_docs.pyruns while preserving strict warning checks in repository CI. This keeps contributor docs builds usable by default without weakening GitHub or GitLab documentation validation.Changes
Checklist
Unreleasedsection.Validation summary
Built the documentation locally with an isolated Warp cache path. The build generated API stubs, ran Sphinx, and completed successfully.
Command:
Summary by CodeRabbit
New Features
WARP_DOCS_OFFLINE=1).Bug Fixes
Documentation