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

first draft PR docs build workflow #1042

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from
Draft

first draft PR docs build workflow #1042

wants to merge 10 commits into from

Conversation

briantist
Copy link
Contributor

This PR adds a workflow that allows for building documentation in PRs. We already have publishing with RTD for releases (tags) and pushes to main, and we have a CI job that runs doctest (which implicitly also ensures that docs are valid since they need to be built).

This workflow is a little different. On a PR, it builds the HTML docsite for both the PR contents and for the target branch (for example main), and then diffs them. If there are any changes, it:

  • publishes a temporary version of the docsite on GH pages (so PR authors and reviewers can see it rendered)
  • posts a comment to PR that contains the list of changed files, diff output, and links to the rendered versions

This workflow uses the pull_request_target trigger which has a few caveats and things to know:

  • the workflow itself is always loaded from the PR's target branch (like main), so the workflow that gets executed is never from the PR itself (this is a necessary security decision)
  • development of the workflow (like in this PR) then is a little tricky:
    • the PR branch should be pushed up to this repo, and not to a fork (so it must be done by a maintainer)
    • then, you can open a new PR (this one can be from a fork) against the first PR's branch as the target, and that will allow the "modified" version of the workflow to run
    • the test PR can be closed when it's done
  • because a PR author without write access can't modify the workflow, that also means that on PRs from new users, these workflows run without someone needing to approve them, which is nice since the feedback is fast

@briantist briantist added documentation documentation updates and/or requests for expanded documentation developer experience Developer setup and experience labels Sep 4, 2023
@briantist briantist self-assigned this Sep 4, 2023
@briantist briantist mentioned this pull request Sep 4, 2023
@codecov
Copy link

codecov bot commented Sep 4, 2023

Codecov Report

Merging #1042 (aa7c063) into main (8e2007e) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1042   +/-   ##
=======================================
  Coverage   85.00%   85.00%           
=======================================
  Files          65       65           
  Lines        3135     3135           
=======================================
  Hits         2665     2665           
  Misses        470      470           

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
developer experience Developer setup and experience documentation documentation updates and/or requests for expanded documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant