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

Remove the pull_request trigger for the documentation workflow #1490

Merged
merged 1 commit into from
Aug 15, 2023

Conversation

inickles
Copy link
Contributor

Issue

The documentation workflow is triggered to run on both all pull requests and pushes to master. The former cannot (and should not) succeed on PRs from the public, even if the workflow is manually triggered by some with write permissions (Oxide). This has caused checks by public PRs to fail, such as #1488 with https://github.com/oxidecomputer/hubris/actions/runs/5838750982/job/15876474521?pr=1488.

Additionally, we probably don't want to trigger the workflow on pull requests, because it'd mean the live GH pages are updated before a PR is accepted. For example: https://github.com/oxidecomputer/hubris/pull/1489/files can be seen on https://oxidecomputer.github.io/hubris/reference/.

Fix

This PR just removes the trigger on pull_reqeusts while keeping the trigger on changes to the master branch, so the GH pages at https://oxidecomputer.github.io/hubris/reference/ should only reflect what's on master.

It might make sense to refine the trigger to run on changes to the doc directory, but this will at least resolve the issues.

Notes

The JamesIves/github-pages-deploy-action action uses the builtin GITHUB_TOKEN by default, which cannot have write permissions to the repo (https://docs.github.com/en/actions/security-guides/automatic-token-authentication#permissions-for-the-github_token).

`documentation`, leaving the trigger for pushes to main.
Copy link
Contributor

@steveklabnik steveklabnik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed! At some point, I standardized our various workflows, and I bet that I introduced this as a bug when doing so. Thank you!

@steveklabnik steveklabnik merged commit 61d1d25 into master Aug 15, 2023
71 checks passed
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