-
Notifications
You must be signed in to change notification settings - Fork 374
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add markdown doc structure and gh-pages workflow #5942
Add markdown doc structure and gh-pages workflow #5942
Conversation
Add docs skeleton for ELM and verify it renders.
Add more basic structure for ELM docs including tech and dev guid subdirs change users-guide to user-guide
Add tech and dev guide skeleton to ELM index
Remove out-of-date eam/doc dir which only had old ChangeLog and ReleaseNotes (no actual docs)
Add EAM documentation skeleton. subdirs, index.md and mkdocs.yml
Add a theme to start with and some useful mkdocs extensions
Add docs skeleton for MOSART and include it in the main docs
Change Intro to Introduction in ELM, EAM and main
Update repo in gh-pages from fork to main repo
You can see what the documentation looks like at: https://rljacob.github.io/E3SM/ If you want to start writing docs, see https://acme-climate.atlassian.net/wiki/spaces/DOC/pages/3924787306/Developing+Documentation |
Need to add more python modules to the pip install command
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good Rob! A couple of suggestions (up to you, we can do that later):
we can make the action conditional to any of the docs folder being modified (no need to re-gen the docs if no docs changed). That said, it's not an expensive thing (and runs on the cloud), so it's fine to update docs at every master update.- we could add the pr-preview action, so we get a preview of the new documentation inside every pr. See this snippet in the scream fork
Edit: I'm striking my first suggestion. It is possible that some packages do not touch their docs folder, and yet require a rebuild of the docs (SCREAM does need a rebuild, if the defaults XML file in the cime_config folder changes). So always rebuilding is the safest choice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless you want to defer some improvements, let's incorporate a few things while here
.github/workflows/gh-pages.yml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we rename this file in anticipation for upstream/downstream merging? How about e3sm-gh-pages? or eam-elm-mosart-gh-pages?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought it had to be named gh-pages.yml. How does github know to run e3sm-gh-pages.yml to update the gh-pages branch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it doesn't matter at all. The name can be anything. Github knows about gh-pages branch from the mkdocs gh-deploy
command
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, GitHub will run every .yml file in workflows individually. They can be linked, etc. and all sorts of interactions. We can also add more jobs inside one single yml file. A lot of things can happen in these actions 😸
Thanks @rljacob!!! 🥳 I left some comments on improving the GitHub actions file. Non-blockers.
I strongly support this. I would advise that we want people to base the docs building from previewing the PRs rather than building locally. In other words, I think we should encourage people to write the docs on the fly so that it is as easy as it gets without having to think or set up stuff locally. To this end, two edits are important:
|
Modify workflow to operate on both PR creation and pushes to master. Update version of checkout action. Co-authored-by: Naser Mahfouz <[email protected]>
Rename gh-pages workflow file to e3sm-gh-pages.yml
Remove submodule checkout in pages workflow. Will be faster.
.github/workflows/e3sm-gh-pages.yml
Outdated
- uses: actions/checkout@v4 | ||
with: | ||
repository: e3sm-project/e3sm | ||
ref: master |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is why the action is failing, we are asking it to pull master no matter what. We don't need that. I will suggest an alternative
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can prob use ${{github.ref}}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I think that may be the default, so no need to specify the ref.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, but there's really no need. We can just rely on the actions/checkout to do the right thing here
.github/workflows/gh-pages.yml
Outdated
workflow_dispatch: | ||
|
||
concurrency: | ||
# Prevent 2+ copies of this workflow from running concurrently | ||
group: e3sm-docs-action |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To get really fancy, you change this to
group: ${{ github.ref }}
so that different PRs can still build at the same time (they deploy to different preview pages anyways)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can also add
cancel-in-progress: true
to the concurrency section, since we prob don't care about finishing to build docs if a commit is pushed...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good point. We will have to adjust these settings especially once we add the previews to avoid users writing previews simultaneously. For now, likely okay as-is
@mahf708 I accepted all your suggested changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one minor thing, and everything should be good to go. We can trust the action to just fetch the right thing
@rljacob if the |
Don't specify which repo/branch to check out. Action will do the right thing. Co-authored-by: Naser Mahfouz <[email protected]>
- uses: actions/checkout@v4 | ||
with: | ||
show-progress: false | ||
fetch-depth: 0 # Needed, or else gh-pages won't be fetched, and push rejected |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At some point we may want to use another action for deployment, which will get rid of the need for fetch-depth: 0
. E.g., in scream repo we use JamesIves/github-pages-deploy-action@v4
, which plays nicely with rossjrw/pr-preview-action@v1
, which we use to deploy preview of PR docs.
As it is now, if running on a PR, this action only builds the docs but does not show it anywhere. We can merge as is, but I would soon work on making a preview of docs available on the PR page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes lets do that in a later PR.
Add infrastructure to build markdown-based documentation located in each components "docs" directory into a single web-page of documentation. Makes use of mkdocs-monorepo-plugin. Also add a skeleton of documentation subdirs and markdown files for EAM, ELM and MOSART. Also add a github action to build and deploy the documentation on e3sm-project.github.io/e3sm whenever master is updated. And an action to build the docs on every PR. [BFB]
Was tested on the rljacob fork. Nothing to test with next. |
Add infrastructure to build markdown-based documentation located in each components "docs" directory into a single web-page of documentation. Makes use of mkdocs-monorepo-plugin.
Also add a skeleton of documentation subdirs and markdown files for EAM, ELM and MOSART.
Also add a github action to build and deploy the documentation on e3sm-project.github.io/e3sm whenever master is updated. And an action to build the docs on every PR.
[BFB]