Closed noelmiller closed 1 month ago
@seanh: Any thoughts on this topic?
@justinmayer I think it's a great idea to let people test if the build succeeds on their PR before merging and trying to deploy it!
I would probably implement it slightly differently than what @noelmiller has done. Here's what I'd try:
Have a .github/workflows/build.yml
workflow that just contains the build
job from the current github_pages.yml
workflow.
Users who want to can then have their own build.yml
caller workflow in their website repos that calls Pelican's build.yml
workflow, and this caller build.yml
workflow can be configured to run on PRs and also to be runnable manually (with workflow_dispatch
).
This can be added to the docs as an optional step for users who'd like to test their build on PRs before merging them.
Have a separate .github/workflows/github_pages.yml
workflow that has a build
job that just calls the build.yml
workflow and then has the deploy
job from the current github_pages.yml
workflow.
In their website repos users would continue to call this github_pages.yml
workflow in the same way that they do currently, with a caller workflow that runs on push to the main
branch or manually.
The github_pages.yml
workflow would have to get the output (the built site) from its call to the build.yml
workflow. Passing outputs between jobs looks like it works pretty much the same was as it does currently with both jobs in the same workflow, see: https://docs.github.com/en/actions/sharing-automations/reusing-workflows#using-outputs-from-a-reusable-workflow
With this approach the deploy
job would not need the if: github.event_name != 'pull_request'
condition from @noelmiller's example.
One advantage of this setup is that if someone ever implements a workflow for deploying to some other hosting service besides GitHub Pages, that workflow could also call the same build.yml
workflow for the build job. So we could one day have several deployment workflows github_pages.yml
, gitlab_pages.yml
, sourcehut_pages.yml
, etc, and these could all call the same build.yml
workflow. (Or if a user wants to add their own foobar_pages.yml
workflow to their own repo rather than adding it to the Pelican repo, it can still call Pelican's build.yml
workflow for the build job.)
Hi @seanh!
I appreciate you reviewing this so quickly and providing awesome feedback. I will see if I can put a PR together with your suggestions and ping you and @justinmayer for review.
Cheers!
Noel Miller
@seanh How does this look as a first pass? https://github.com/getpelican/pelican/pull/3404
If we want to keep the workflows running in the same way, I had to use inputs for both workflows.
Here is what I have in my website repo as an example: https://github.com/noelmiller/noelmiller.github.io/tree/main/.github/workflows
Here is a successful build: https://github.com/noelmiller/noelmiller.github.io/actions/runs/11243940338 Here is a successful deploy: https://github.com/noelmiller/noelmiller.github.io/actions/runs/11243960459
Hi @noelmiller, https://github.com/getpelican/pelican/pull/3404 looks like the right approach to me at a glance. It may be a few days before I can test and review it but I'll try to get to it soon
Feature Request
Hi There!
When building my pelican site in a PR, I noticed that the action the pelican team publishes will push the deployment to github pages no matter what. My suggestion would be to decouple the deployment of the site and the building of the site from each other. This way, you can skip the publish job when running the build workflow from a PR and can ensure your build is successful in a PR before merging your change into your main branch.
Here is an example of what I did in my pelican.yml: https://github.com/noelmiller/noelmiller.github.io/blob/main/.github/workflows/pelican.yml
Line 15 is the important part: https://github.com/noelmiller/noelmiller.github.io/blob/e8322ab315dae4a488ba403fd2b78ddfacedee41/.github/workflows/pelican.yml#L15
Here is what I did in my fork: https://github.com/getpelican/pelican/compare/main...noelmiller:pelican:main
I'm filing an issue because I'm not sure if this is something you guys would want? It would just require removing that section and including documentation on what a user is supposed to do.
Thoughts?
Thanks,
Noel Miller