Skip to content
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

Optionally upload docs to GH Pages #8

Merged
merged 2 commits into from
Aug 5, 2024

Conversation

lkubb
Copy link
Member

@lkubb lkubb commented Aug 1, 2024

Adds a switch for automatically deploying the latest built docs to GitHub Pages.

This approach is very basic (not versioned), but helps with solving the immediate problem of not having any docs on most published extensions. I have been using this on https://github.com/salt-extensions/saltext-vault for a while now (https://salt-extensions.github.io/saltext-vault/) and am mostly satisfied (for a start).

Testing centralized workflows is hard, so I hope this works as I imagine (otherwise I'll do a quick fix).

Versioning notes

When using GH Pages instead of readthedocs, versioning support would require either (better):

or (more basic):

  • using the gh-pages branch and peaceiris/actions-gh-pages
    to be able to deploy to subdirectories. The implementation via
    actions/deploy-pages always replaces the directory root.

@lkubb
Copy link
Member Author

lkubb commented Aug 2, 2024

@max-arnold Do you agree with this in principle? I know there have been a lot of discussions around the docs. In contrast, this is a very bare-bones MVP, but it gives contributors a frictionless way of hosting them immediately. As we all know, the current default state of having no docs is suboptimal.

@max-arnold
Copy link
Contributor

I'm not very familiar with GH workflows, so please excuse me if the review is not correct or I haven't spotted a syntax issue.

  • I agree that GH pages is the simplest way to get the docs published
  • Is there a way for a contributor to preview an extension docs without merging a PR? Maybe we can allow running the workflow outside of the salt-extensions org, or it would be bad from SEO perspective (duplicate content)? Or maybe it will be more practical to wait for the native preview feature to be available?
  • Which prerequisite jobs are mandatory for pages to be published? Is it worth requiring the tests passing?
  • Should the deploy-docs step be required by the set-pipeline-exit-status?

Feel free to merge as-is, we can iterate on this later and I really need to go through the whole publishing process myself.

@lkubb
Copy link
Member Author

lkubb commented Aug 3, 2024

I'm not very familiar with GH workflows, so please excuse me if the review is not correct or I haven't spotted a syntax issue.

No worries, those are easy to fix in "prod". =)

* Is there a way for a contributor to preview an extension docs without merging a PR?

One could download the html-docs artifact (works on PRs as well) and display it in the browser.

Maybe we can allow running the workflow outside of the salt-extensions org

Unsure what the permissions are currently and whether reusable workflows can be called from outside an org in principle, but...

bad from SEO perspective (duplicate content)?

... the docs can be built via nox -e docs or downloaded as the html-docs artifact already, only the upload to GH Pages is gated.

* Which prerequisite jobs are mandatory for pages to be published? Is it worth requiring the tests passing?
* Should the `deploy-docs` step be required by the `set-pipeline-exit-status`?

Yeah, I've been thinking about that. It's a bit cumbersome to require a step that could be skipped, but possible with workarounds. Agree the tests should probably be passing.

Feel free to merge as-is, we can iterate on this later and I really need to go through the whole publishing process myself.

👍 I'll modify to require the tests for the docs publish job at least.

@lkubb lkubb merged commit d76fdb3 into salt-extensions:main Aug 5, 2024
2 checks passed
@lkubb lkubb deleted the feat/gh-pages-docs branch August 5, 2024 06:25
@lkubb
Copy link
Member Author

lkubb commented Aug 5, 2024

Alright, so this worked out, but it requires a copier update for all calling repositories because id-token: write and pages: write permissions are required now, even if the corresponding job never actually runs.

It's not too bad since this was true before regardless (because the docs build was broken with the release of importlib_metadata v8). I really wish we already had Renovate support for Copier already though.

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