Main loop plugins #119
Main loop plugins #119
Conversation
oliver-sanders
force-pushed
the
main-loop-plugins
branch
from
c31efd4
to
492bd3c
Compare
There was a problem hiding this comment.
Nice, very helpful for reviewing the Cylc Flow PR for plugins. The documentation is in the code of Cylc Flow , so will comment there if I find anything.
Approved! Only minor issue were some links that are not working @oliver-sanders , like auto_restart. For some reason I didn't get the module in my User Guide I think? This link (from my generated singlehtml file) works:
file:///home/kinow/Development/python/workspace/cylc-doc/doc/8.0a1/singlehtml/index.html#module-cylc.flow.main_loop
But every link for one of the modules in this package result in 404, like
file:///home/kinow/Development/python/workspace/cylc-doc/doc/8.0a1/singlehtml/index.html#module-cylc.flow.main_loop.auto_restart
I re-created the venv, and used pip install -e .[all] in Cylc Flow (from your other branch) and again for cylc-doc. Not a blocker IMO 👍 great stuff.
| such they can often trigger on time even if the suite is delayed to | ||
| the point that downstream tasks are late due to their dependence on | ||
| previous-cycle tasks that are delayed. | ||
| See :py:mod:`cylc.flow.main_loop.auto_restart` for details. |
There was a problem hiding this comment.
Link not working for me (checked out Cylc Flow PR for main loop, then pip install -e . with my cylc-doc's venv).
There was a problem hiding this comment.
Ah, yeah, so these :py: thinggies are code references which only work if the code being referenced is auto-documented. The documentation system uses Python introspection so the thing you're documenting must be installed.
|
I think from a similar Simmental by Tim on the Sphinx-Cylc-extensions repo, that the Sphinx autosummary plugin is not simpatico with singlehtml, at lest when autobuilding. Simple solution, don’t build singlehtml, perhaps unhelpful. Will look and see if anyone has raised this with sphinxdoc at some point. |
|
In the above there are phone typos for the words, “comment” and “compatible” that are so brilliant that I’m not going to edit the comment to fix them. |
|
Links working on the |
There was a problem hiding this comment.
Looks good to me..
However, as @kinow found;
- I had to install cylc-flow with the entry-points branch checked out..
- And the "Next/Previous" navigation doesn't cycle through all the built-in plugins.. Although they are available via the table: file:///home/sutherlander/cylc/cylc-doc/doc/current/html/user-guide/plugins/main-loop/index.html#built-in-plugins
⭐️
Replied on Riot with a video on my branch. |
|
Whilst we're all here, how do you feel about this approach of burying large quantities of documentation in the Cylc Flow codebase. When you're documenting functions, classes, etc it makes good sense. When you are writing more user-guide oriented narrative, however, perhaps not quite perfect. |
Fine either way I think. Only possible disadvantage I can think of, is if a user finds an error in the docs. That would require fixing in Cylc Flow, not in cylc-doc if that's auto-generated. Then probably the CI would take care to re-publish latest docs with the fix. For a user contributing, I think it's harder to preview the changes. But it could be easier to maintain for developers this way (i.e. code and docs are synced). |
|
A couple of disadvantages from me:
I'm in two minds, but for niche dev stuff that's tied to an API, perhaps best to have the docs in the code. |
At the same time forces developers to fix the doc in their code, a good side-effect I think. |
|
Two approvals, LGTM |
Companion to cylc/cylc-flow#3492
Document the newfangled main-loop plugins.