add first attempt at page building tests for CI - #361
Conversation
| @@ -0,0 +1,19 @@ | |||
| name: Build Jekyll site | |||
| on: | |||
| pull_request: | |||
There was a problem hiding this comment.
I am used to limiting this to specific branches. However, if you aren't deploying anything I think that won't be necessary here.
| pull_request: | ||
| workflow_dispatch: | ||
|
|
||
| jobs: |
There was a problem hiding this comment.
Best practice is to be explicit with the permissions:
| jobs: | |
| jobs: | |
| permissions: | |
| contents: read |
This is probably not essentially, but does protect against GH changing default permissions in the future. reference
|
|
||
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@v7 |
There was a problem hiding this comment.
More of an FYI: this style of version specification is being discouraged for security nowadays. This is because version numbers on GH actions are not immutable, and could be maliciously changed/released. The workaround is to specific full git SHAs.
However, this is very tedious to keep up with, and really necessitates dependabot managing actions. Given that these are all 'actions` maintained I think the risk of a malicious release is rather low, so I think in this case it's fine as is. Just an FYI. Reference.
magnoxemo
left a comment
magnoxemo
left a comment
There was a problem hiding this comment.
Should we also update the README.md with the new build instructions (downloading the artifact and building it locally using python)? We can keep the Docker option for folks who want to build it locally first and then open a PR.
Not necessarily, being able to build locally is still helpful, and how it ought to be done for large updates. |
I think we should add something to the README, but leave the previous info as @MicahGale suggested |
There was a problem hiding this comment.
I'm not the most familiar with GitHub Actions, but my only thought is it would be nice to test that this works before merging, but it seems like a chicken and egg game where we can't test new PRs with this until it's merged. I think once people are happy with this, we can merge, test, and make additional PRs as needed.
Also, would be cool to have a bot that auto comments the built link like how https://github.com/moosebuild does for docs/coverage/etc.
The test that was run on this PR demonstrates it |
I am looking for a way to post a link to the artifact in the comments to make it easier to find |
Yeah, I'm looking at the test Summary but is the |
Hmmmm.... maybe I need to set the time that it is saved |
You can change this in repo settings outside the workflow. Maybe 30 days would be good? https://docs.github.com/en/organizations/managing-organization-settings/configuring-the-retention-period-for-github-actions-artifacts-and-logs-in-your-organization |
Added retention policy for uploaded artifacts. This action overrides the organization default (currently 90)
Added section on CI testing of file rendering with instructions for viewing artifacts.
Add a link to the artifact in the comments
Added permissions for contents and pull-requests in build job. Removed link job and integrated its step into build.
This action is deprecated/uses deprecated code… many similar actions seem to have the same problem.
|
|
||
| When a pull request (PR) is submitted for this repository, an action will automatically attempt to build the | ||
| pages and upload an artifact of that process as part of the test, named `github-pages`. The artifact can be | ||
| found by viewing the results of the test, clicking on the “Summary” and scrolling to the bottom of the page. |
There was a problem hiding this comment.
Another option maybe? with out scrolling through CI summary.
4 participants
Add a CI workflow that builds the pages and saves them as an artifact that is a ZIP file of the full built site. That artifact can be downloaded, unpacked and served locally (
python3 -m http.server) and then reviewed on a local port.The artifact is found by clicking on the test, navigating the the test "Summary" and scrolling down.