Infra: Add GH action for docs build passing#676
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughGitHub Actions workflow renamed and split into build (produces Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant Runner as Workflow Runner
participant Artif as Artifact Storage
participant GCS as Google Cloud Storage
GH->>Runner: trigger (workflow_dispatch / release / pull_request on main)
Runner->>Runner: run build job\nSPHINXOPTS="-W" make html
Runner->>Artif: upload `docs-html` artifact
Runner->>Runner: start deploy job (depends on build)
Runner->>Artif: download `docs-html` artifact
Runner->>GCS: gsutil rsync ./docs-html/ gs://<bucket>/path
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
Adds a CI guardrail to ensure the Sphinx documentation successfully builds on every PR to main, helping catch documentation regressions (e.g., missing/invalid sections) before merge.
Changes:
- Introduces a new GitHub Actions workflow to build Sphinx docs on
pull_requesttomain. - Installs the project package plus
docs/requirements-docs.txtdependencies before runningmake htmlwith warnings treated as errors (SPHINXOPTS="-W").
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 281435e3-70e9-40f6-8c3f-3c9958408fd3
📒 Files selected for processing (1)
.github/workflows/docs-build-check.yml
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Agent
- GitHub Check: build (3.10)
- GitHub Check: build (3.9)
- GitHub Check: build (3.13)
- GitHub Check: build (3.11)
- GitHub Check: build (3.12)
🧰 Additional context used
🪛 actionlint (1.7.11)
.github/workflows/docs-build-check.yml
[error] 21-21: the runner of "actions/setup-python@v4" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/docs-publish.yml (1)
41-46: Addif-no-files-found: errorto fail fast if artifact path is missing.The default behavior (
warn) allows the workflow to continue even if the docs build directory is absent, deferring failure to the deploy stage. Useerrorto catch the issue immediately in thebuildjob.♻️ Proposed change
- name: Upload build artifact if: github.event_name == 'release' || github.event_name == 'workflow_dispatch' uses: actions/upload-artifact@v4 with: name: docs-html path: docs/build/html + if-no-files-found: error
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4869f10d-12f6-4542-a8cf-2aef12ffa411
📒 Files selected for processing (1)
.github/workflows/docs-publish.yml
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build (3.10)
- GitHub Check: build (3.12)
- GitHub Check: build (3.9)
🔇 Additional comments (2)
.github/workflows/docs-publish.yml (2)
7-9: Good separation of validation vs deployment paths.Running
buildonpull_requestwhile keepingdeploygated torelease/workflow_dispatchcleanly enforces docs build health without accidental publishing.Also applies to: 12-13, 48-52
38-39: Strict Sphinx warning handling is a strong quality gate.Using
SPHINXOPTS="-W"is the right move for catching doc regressions early.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/Makefile (1)
20-21: Remove redundant warning-as-error flags.Both
-Wand--exception-on-warningcause Sphinx to treat warnings as errors. Since the project uses Sphinx 9.1.0, both are supported, but using both is redundant. Choose one:
- Use just
-W(simpler, shorter syntax)- Use just
--exception-on-warning(explicit intent)If CI already passes
SPHINXOPTS="-W", the hardcoded flags here add another layer of redundancy.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8067f4d0-f422-436c-bc16-ea1930a0e4c1
📒 Files selected for processing (6)
dagshub/common/cli.pydagshub/data_engine/annotation/voxel_conversion.pydocs/Makefiledocs/requirements-docs.txtdocs/source/conf.pydocs/source/reference/metric_logging.rst
✅ Files skipped from review due to trivial changes (4)
- docs/source/reference/metric_logging.rst
- dagshub/common/cli.py
- docs/requirements-docs.txt
- dagshub/data_engine/annotation/voxel_conversion.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build (3.13)
- GitHub Check: build (3.10)
- GitHub Check: build (3.9)
- GitHub Check: build (3.11)
- GitHub Check: build (3.12)
🧰 Additional context used
🧬 Code graph analysis (1)
docs/source/conf.py (1)
dagshub/streaming/filesystem.py (1)
path(1119-1124)
🪛 Ruff (0.15.6)
docs/source/conf.py
[error] 22-22: Variable copyright is shadowing a Python builtin
(A001)
🔇 Additional comments (4)
docs/source/conf.py (3)
11-11: LGTM!The
typesimport is correctly added for creating the synthetic module below.
16-19: LGTM!Good fix for the doc build environment. The synthetic module injection is correctly placed before any project imports would occur, and the comment explains the intent well.
Minor note: if other attributes from
dagshub.common.environmentare accessed during doc builds, they may raiseAttributeError. This should be acceptable for current usage, but keep in mind if future doc build issues arise.
22-22: LGTM!Copyright year correctly updated.
Regarding the static analysis hint about shadowing the
copyrightbuiltin: this is expected and intentional.copyrightis a standard Sphinx configuration variable name, and this pattern exists in all Sphinxconf.pyfiles. The warning can be safely ignored.docs/Makefile (1)
23-24: LGTM!Using
-Wforsphinx-autobuildis appropriate and ensures consistent warnings-as-errors behavior during local development.
This should prevent problems with sections disappearing from documentation.