Skip to content

Docs that can build - #25

Open
thc1006 wants to merge 2 commits into
developfrom
fix/docs-that-cannot-build
Open

Docs that can build#25
thc1006 wants to merge 2 commits into
developfrom
fix/docs-that-cannot-build

Conversation

@thc1006

@thc1006 thc1006 commented Jul 29, 2026

Copy link
Copy Markdown
Member

The docs job has failed on every run since it was added. Five runs, back to 2026-07-26, all red:

failure  master   2026-07-29  Merge pull request #23 from ARRC-Rocket/develop
failure  develop  2026-07-29  REL: v1.13 hotfix, actuator argument validation (#19)
failure  master   2026-07-26  REL: RocketPy v1.13, sensor noise rng, pytests
failure  develop  2026-07-26  ...
failure  develop  2026-07-26  ...

sphinx-build -W turns warnings into errors and there are four. All four come from this fork's own additions, not from upstream, which is why they arrived with the job rather than being caught by it.

Three of them are one stray line, copied three times

add_thrust_vector_control, add_roll_control and add_throttle_control describe the controller arguments as a numbered list. Item 7 ends with a loose interactive_objects at one space less than the continuation lines above it:

add_air_brakes (upstream)              add_thrust_vector_control (fork)
 16  7. `sensors` (list): ...           16  7. `sensors` (list): ...
 19     rocket. The most recent         19     rocket. The most recent
 19     with the ``sensor.mea...        19     with the ``sensor.mea...
 19     listed in the same order        19     listed in the same order
 16  8. `environment` ...               18  ``interactive_objects``   <- this

docutils sees the indent go 19 -> 18 and reports a definition list ending without a blank line. The line carries no meaning where it is; upstream's add_air_brakes has the same item 7 and moves straight on to item 8. git log -L traces all three to 87e8f86, the RocketPy 1.12 merge, so they predate the job that reports them and did not come from #19.

Removing them puts these three docstrings back in step with the method they were copied from, which is also the smaller divergence from upstream.

The fourth

docs/examples/halcyon_flight_sim_active_control.ipynb sits in docs/examples/ and in no toctree, so Sphinx builds it and then reports that nothing links to it. It is this fork's own example of the feature the fork exists for, so it goes into the list next to the flight it varies rather than into an exclude pattern.

Verified

Per docstring, running napoleon and docutils the way Sphinx does, against this branch and against origin/develop:

AFTER (this branch)                     BEFORE (origin/develop)
  add_thrust_vector_control  0            add_thrust_vector_control  1  line 28
  add_roll_control           0            add_roll_control           1  line 28
  add_throttle_control       0            add_throttle_control       1  line 28
  add_air_brakes             0            add_air_brakes             0

Same line number and same message as the job reports. add_air_brakes is the control: it is upstream's, it is clean in both, and it stays clean.

I could not finish a full local build to confirm the whole run goes green. An unrelated notebook fetches a live weather forecast and the file it reaches no longer covers the date it asks for, so the build stops here for a reason CI does not have.

And the job will not run on this pull request. docs.yml triggers on pull_request with branches: [master] and on pushes to master, so a PR into develop never starts it. That is also why this went unnoticed: the only runs are the ones a develop-to-master PR sets off, so a docs regression lands on develop and is first seen at the release merge, which is what happened with #19 and #23. Worth considering whether that trigger should include develop, but that is a separate change from the four warnings, so it is not in here.

Concretely: green checks on this PR say nothing about the docs. The proof offered is the per-docstring measurement above.

The docs job has failed on every run since it was added: five runs, back
to 2026-07-26, all red. `sphinx-build -W` turns four warnings into
errors, and all four come from this fork's own additions rather than
from upstream.

Three are one stray line, copied three times. `add_thrust_vector_control`,
`add_roll_control` and `add_throttle_control` describe the controller
arguments as a numbered list, and item 7 ends with a loose
``interactive_objects`` at one space less than the continuation lines
above it. docutils reads that as a definition list ending without a
blank line. The same item in upstream's `add_air_brakes` does not have
it, so removing it puts these three back in step with the method they
were copied from.

Measured per docstring, running napoleon and docutils the way Sphinx
does. Before: those three warn at line 28, `add_air_brakes` does not.
After: none of the four warn.

The fourth is `halcyon_flight_sim_active_control.ipynb`, which is in
`docs/examples/` and in no toctree, so Sphinx builds it and then reports
that nothing links to it. It is this fork's own example of the feature
the fork exists for, so it goes in the list next to the flight it varies
rather than into an exclude.

Not verified by a full local build: an unrelated notebook fetches a live
weather forecast and the file it reaches no longer covers the date it
asks for, which stops the build here for a reason CI does not have.

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 29, 2026 15:58

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

`docs.yml` triggered on a base of master only, so the only pull request
that ever started it was develop into master. A docstring that does not
build therefore lands on develop unremarked and first fails the merge
that was meant to ship it, which is what #19 and #23 hit and why the four
warnings in the commit before this one went unnoticed for three days.

develop as well. The same paths filter, so it still only runs when
something it reads has changed.

This also makes the commit before it self checking: without this, a pull
request into develop cannot start the job that would prove the warnings
are gone, and the only evidence would be a measurement in the
description.

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
@thc1006

thc1006 commented Jul 29, 2026

Copy link
Copy Markdown
Member Author

Ready for review. Nine checks green, and the one that matters is build-docs (3.12), which is passing here for the first time since the job was added.

That is only true because the trigger change is in the same PR. docs.yml fired on a base of master only, so a pull request into develop could not start the job that would prove the warnings were gone, and the evidence would have been a measurement in the description. Now the PR checks itself.

That trigger is also why nobody saw this for three days: the only run was the develop into master pull request, so a docstring that does not build lands on develop unremarked and first fails the merge that was meant to ship it.

The four warnings are all this fork's own additions rather than upstream's, which is why they arrived with the job instead of being caught by it. Three are one stray line copied three times, traced with git log -L to 87e8f86, the RocketPy 1.12 merge. Removing it puts those docstrings back in step with upstream's add_air_brakes, which is the smaller divergence as well as the fix.

@thc1006
thc1006 requested a review from zuorenchen July 29, 2026 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants