-
Notifications
You must be signed in to change notification settings - Fork 182
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
GH Actions: no setup deployment using GITHUB_TOKEN #310
Conversation
16b1a82 properly deployed in this build and triggered the GitHub Pages build despite using GITHUB_TOKEN on a public repo! |
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.
If I understand correctly, the GITHUB_TOKEN is created as soon as GitHub Actions are enabled without any further user intervention. So users can completely avoid creating a deploy key on the command line, which greatly simplifies the setup. That's a great improvement.
SETUP.md
Outdated
### Deploy key | ||
|
||
Generate a deploy key so CI can write to the repository. | ||
Deployment on Travis CI requires a SSH Deploy Key. | ||
Previousely, GitHub Actions also required an SSH Deploy Key, but now GitHub supports using `secrets.GITHUB_TOKEN`. |
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.
Previousely
-> Previously
Do we want to explain the differences between GITHUB_TOKEN, an SSH deploy key, and a GitHub personal access token in terms of permissions? These terms can be confusing for people who aren't experienced with continuous integration. Others may mix up this GITHUB_TOKEN with the broader personal access token.
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 updated the SETUP extensively, hopefully a bit clearer on these points although still don't go into too much detail.
Yes. Note that previously the approach in this PR did not work because GITHUB_TOKEN deployments did not trigger GitHub Pages builds for public repositories. As reported by a community member in peaceiris/actions-gh-pages#9 (comment), this no longer seems to be the case. However, there has been no official GitHub announcement about the change, so there is a risk that it will be temporary. |
Therefore existing users with MANUBOT_SSH_PRIVATE_KEY will continue to use it, in case the GITHUB_TOKEN method starts failing in the future
eddd939 deployed properly in this build. eddd939 uses I'll try to improve the setup docs. After that I think this PR will be ready. |
731d12b
to
af3cc5b
Compare
@agitter ready for re-review |
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 to me. I have a couple small optional questions and comments.
SETUP.md
Outdated
- [Cloning the manubot/rootstock repository to create a new manuscript](#cloning-the-manubot-rootstock-repository-to-create-a-new-manuscript) | ||
# Table of contents | ||
|
||
- [Table of contents](#table-of-contents) |
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.
We could remove this line from the table of contents even though it is technically a section
Manubot supports the following services: | ||
Manubot supports the following CI services: | ||
|
||
| Service | Default | Artifacts | Deployment | Config | Private Repos | |
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.
The table is great. This will be very helpful now that we have multiple CI options
Notes on table fields: | ||
|
||
- **Default**: Whether the following uncollapsed setup instructions enable the service by default. | ||
- **Artifacts**: Manuscript outputs that are saved alongside the CI build logs. |
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.
May want to add something about this being useful for previewing changes in the manuscript before merging a pull request.
GitHub Actions will deploy by default without any additional setup. | ||
Travis CI will only deploy if an SSH Private Key is provided. | ||
To avoid deploying a manuscript multiple times, disable GitHub Actions before providing an SSH Private Key to Travis. | ||
- **Config**: File configuring what operations CI will perform. |
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.
Should we add that deleting this file will disable the CI service? There are other steps needed to enable the service, however, so that may be more confusing than helpful.
SETUP.md
Outdated
|
||
Generate a deploy key so CI can write to the repository. | ||
Deployment on Travis CI requires a SSH Deploy Key. |
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.
Used an SSH
elsewhere
|
||
### GitHub Actions | ||
|
||
GitHub Actions is the recommended default CI service because it requires no additional setup. |
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.
It looks like GitHub Actions is enabled by default, right? The owner has to explicitly disable it in the repository settings. If that's accurate, these instructions make sense.
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.
In the most common case, adding the workflow file should be enough to enable actions. However, it is possible that actions could be disabled at the:
- repo level
- organization level
Not sure how common this will be.
merges manubot/rootstock#310 Previously secrets.GITHUB_TOKEN did not work for deployment because the commits it pushed to `gh-pages` were unable to trigger a GitHub Pages build on public repositories. However, this issue seems to have been quietly resolved. There has been no official GitHub announcement about the change, so there is a risk that it will be temporary. Deployment now uses MANUBOT_SSH_PRIVATE_KEY over MANUBOT_ACCESS_TOKEN when both are set. The idea is that if GitHub's support for GITHUB_TOKEN is temporary (since no official commitment yet), then users who have configured MANUBOT_ACCESS_TOKEN will not be affected. Only users who did not set up MANUBOT_SSH_PRIVATE_KEY will be affected, but they could then set up MANUBOT_SSH_PRIVATE_KEY without having to upgrade Manubot Rootstock to get deployment to work.
Evaluates whether we can use
secrets.GITHUB_TOKEN
for deployment. Testing the PR in the repository https://github.com/dhimmel/rootstock-github-token.Also noting https://github.com/dhimmel/rootstock-actions-deploy to test using
peaceiris/actions-gh-pages
. However, I think that using our existingdeploy.sh
is less disruptive for now.