Reusable Documentation Workflows #17
Conversation
Create two reusable workflows, one each for Dart and Rust to build and deploy their docs using GitHub Pages.
beauremus
requested review from
cnlklink,
jacob-curley-fnal,
mgonzal1,
mguzman04 and
rneswold
| deploy: | ||
| environment: | ||
| name: github-pages | ||
| url: ${{ steps.deployment.outputs.page_url }} |
There was a problem hiding this comment.
Both workflows reference this steps.deployment.outputs.page_url variable. I think I'm missing where that comes from. Is that set by some other workflow?
There was a problem hiding this comment.
Great catch! I used the example from the Action readme, but I don't think we need this anymore. Will push an update shortly.
There was a problem hiding this comment.
I added an id: to the step. So, this output is generated by the Action.
What I'm not sure of is whether the environment needs to exist before being requested.
https://docs.github.com/en/actions/how-tos/deploy/configure-and-manage-deployments/manage-environments
There was a problem hiding this comment.
I'll merge after you approve
There was a problem hiding this comment.
I think the environment is set before the steps execute. So your url variable might just be empty
There was a problem hiding this comment.
Does the deploy-pages action require a url environment variable? Is that how that works?
There was a problem hiding this comment.
This is part of the README docs, but there's no further explanation.
https://docs.github.com/en/actions/how-tos/deploy/configure-and-manage-deployments/control-deployments
This doc doesn't have it. So, let's remove the complexity until it's better understood.
There was a problem hiding this comment.
Looking more closely at the actions/deploy-pages README, it seems like maybe the purpose is to kick out an auto-generated URL somewhere that a person could see it? Like, maybe the action comes up with the URL internally, so you'd need to have it reported into the environment variable in order to know where to look for the deployed pages?
There was a problem hiding this comment.
I'm not sure there's some magic going on for sure. Without diving into the Action code, I'm not sure.
jacob-curley-fnal
left a comment
jacob-curley-fnal
left a comment
There was a problem hiding this comment.
Just had that one clarification question, but otherwise looks solid!
| needs: build | ||
| steps: | ||
| - name: Deploy to GitHub Pages | ||
| id: dpeloyment |
There was a problem hiding this comment.
may want to check spelling here
There was a problem hiding this comment.
We need a linter/spell checker 😳
The url field in the environment and the id: deployment from the actions/deploy-pages@v4 step are no longer necessary as the action automatically sets the page URL. This commit cleans up these redundant configurations in both the Dart and Rust documentation deployment workflows.
| permissions: | ||
| pages: write # Needed to deploy | ||
| id-token: write # Needed for auth |
There was a problem hiding this comment.
Probably need a contents: read entry as well. I'm getting errors when trying to call this, as default behavior is to only set the permissions that are provided here, and to try to use the lowest level permissions possible. My suspicion is that GitHub sees this as us "downgrading" the permissions from the calling workflow, since read is not specified. So it's dropping the read permissions and can't see the code in the repo anymore.
There was a problem hiding this comment.
This will apply to all workflows. Right?
There was a problem hiding this comment.
Any workflow that specifies permissions at this level, yes. The integration and deployment workflows just expect to have their permissions provided. They don't have any special permissions block like this.
There was a problem hiding this comment.
Got a successful deployment! Only problem is that the permissions on the pages site are jank, so I can't access the actual docs 😆 Might be a problem at the repo level tho, or with the org permissions
There was a problem hiding this comment.
Need to figure out the best way to consistently handle the URLs. I found the docs here https://studious-spoon-gz98yym.pages.github.io/reference/rust_env_var_lib/index.html 😅
Not great
There was a problem hiding this comment.
Need to figure out how to resolve the snake case package name for this step:
cp -r target/doc/* ./docs/reference
It should be
cp -r target/doc/rust_env_var_lib/* ./docs/
rneswold
left a comment
rneswold
left a comment
There was a problem hiding this comment.
This is awesome, Beau! I love having our docs available!
|
Should we have it run a docs check pass in the PR and generate the docs when merging? (Is that what it does now?) |
|
Ran into an issue while trying to use this in |
My first pass attempt at automating source documentation for Dart and Rust.