Add documentation for zppy-interfaces#51
Conversation
|
To generate docs: Docs are then available at https://web.lcrc.anl.gov/public/e3sm/diagnostic_output/ac.forsyth2/zi_docs/html/ |
|
@copilot I added a developer guide. Can you make |
Addressed in eb6b62f. Added a new |
There was a problem hiding this comment.
Pull request overview
This PR introduces an initial Sphinx documentation scaffold for zppy-interfaces,
splitting guidance by audience (user vs. developer) and documenting the CLI entry
points for global_time_series and pcmdi_diags.
Changes:
- Add Sphinx docs scaffold (config, Makefile, templates, and shared styling).
- Add audience-based docs structure (home page + user/developer guide indices).
- Add detailed user/developer pages for
zi-global-time-seriesand PCMDI-related
CLIs, and re-enable docs publishing in CI.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/source/index.rst | Adds docs home page with audience-based navigation. |
| docs/source/user_guide.rst | Adds user guide landing page and toctree. |
| docs/source/developer_guide.rst | Adds developer guide landing page plus testing instructions (needs directive indentation fix). |
| docs/source/global_time_series.rst | Adds user-facing runtime guide and parameter reference for zi-global-time-series. |
| docs/source/developer_global_time_series.rst | Adds developer-facing module/flow map for global_time_series. |
| docs/source/pcmdi_diags.rst | Adds user-facing runtime guide and parameter references for PCMDI-related CLIs. |
| docs/source/developer_pcmdi_diags.rst | Adds developer-facing module/flow map for pcmdi_diags. |
| docs/source/conf.py | Adds Sphinx configuration (theme, sidebar, version extraction). |
| docs/source/_templates/layout.html | Adds template override to include custom CSS. |
| docs/source/_templates/versions.html | Adds version switcher sidebar template for multiversion builds. |
| docs/source/_static/style.css | Adds shared CSS tweaks (content width). |
| docs/Makefile | Adds Sphinx Makefile wrapper. |
| .github/workflows/build_workflow.yml | Re-enables docs publishing job (currently has a YAML syntax error and workflow logic/pinning issues). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@copilot Please implement the suggestions from the code review |
forsyth2
left a comment
There was a problem hiding this comment.
I read through the rendered docs (latest: https://web.lcrc.anl.gov/public/e3sm/diagnostic_output/ac.forsyth2/zi_docs_review/html/index.html). They look good to me. For perfect accuracy, I'd need to go through the code line-by-line and compare but I think the read-through should be sufficient.
I reviewed the code files in the diff for the non-rst files as well.
|
@chengzhuzhang @zhangshixuan1987 Do either of you have any objections if I merge these documentation updates? Let me know if you'd like to review them first. For reference, My hope is that going forward, relevant doc changes will be included in |
|
@copilot Can you please give directions for ensuring the docs are actually built after we merge this PR? zppy's documentation has these directions: |
Acknowledged: I will only provide a comment (no commit, no PR summary changes). After merge, docs are built and published automatically by GitHub Actions:
For release tags, docs are also published by the CI/CD Release Workflow on GitHub release publish. |
|
https://web.lcrc.anl.gov/public/e3sm/diagnostic_output/ac.forsyth2/zi_docs_review/html/index.html is still the latest rendering since the only code change since then has been 6b35ba4, which only touched the GitHub workflow. |
forsyth2
left a comment
There was a problem hiding this comment.
I've done a read-through of the latest rendered docs.
I've also confirmed there have been no merges to https://github.com/E3SM-Project/zppy-interfaces/commits/main/ since I started this PR, so there's no need to document later code changes, as we had to do in E3SM-Project/zppy#839.
I think this is ready to merge. Since this is zppy-interfaces's first documentation, we'll have to confirm the docs workflow works properly and actually renders the docs. To include them on docs.e3sm.org we'll have to make a PR to E3SM-Project.github.io, specifically, docs/index.md and mkdocs.yaml. Alternatively, we could just make a PR to update the zppy docs to point to them, which is likely sufficient considering virtually all users will be interacting with zppy-interfaces via zppy.
|
The docs building action failed with: Based on Claude's suggestions, I ran: That then launched a new action. That page says the link is https://docs.e3sm.org/zppy-interfaces/, but that gives a 404 error. It looks like the base link https://e3sm-project.github.io/zppy-interfaces does already auto-redirect to https://docs.e3sm.org/zppy-interfaces/, again giving the 404 error. |
|
Iterating with Claude more, I merged #52, which launched this action, which completed successfully. https://docs.e3sm.org/zppy-interfaces/, however, still gives a 404. https://docs.e3sm.org/zppy-interfaces/_build/html/main/index.html also gives a 404. |
git checkout gh-pages
touch .nojekyll
git add .nojekyll
git commit -m "Add empty .nojekyll file"
git push upstream gh-pages # Fails
git fetch upstream gh-pages
git rebase upstream/gh-pages
git log --oneline | cat
# a92816a Add empty .nojekyll file
# 07bdba5 Update documentation # (Automated GitHub commit)
# ce7fb05 Initialize gh-pages branch
git push upstream gh-pagesThat launches this action. That completes successfully, but both https://docs.e3sm.org/zppy-interfaces/ and https://docs.e3sm.org/zppy-interfaces/_build/html/main/index.html still give a 404. |
|
I switched to Copilot Chat, which added this commit, which simply adds a file |
Summary
Objectives:
zppy-interfaces-- covering user guide and developer guide for both major components:global_time_series&pcmdi_diagsIssue resolution:
Select one: This pull request is...
Small Change