ci: modernize workflows to support fork pr previews #2116
Conversation
splits workflow to enable ipfs deployments for all pull requests including those from forks - build.yml: builds site and uploads artifacts - deploy.yml: handles ipfs deployment via workflow_run - uses ipfs-deploy-action@b491fdc with workflow_run support
🚀 Build Preview on IPFS ready
|
.github/workflows/build.yml
Outdated
| with: | ||
| ref: ${{ github.event_name == 'pull_request_target' && github.event.pull_request.head.sha || github.sha }} | ||
| # No ref parameter needed - uses correct ref automatically: | ||
| # - For PRs: merge commit or PR head |
There was a problem hiding this comment.
How is it decided if it's the merge commit or PR head?
There was a problem hiding this comment.
According to the docs, it defaults to the merge commit, which may seem confusing, because it doesn't reflect the actual HEAD state of the PR branch. Instead, it contains PR changes PLUS any new commits that landed on main since the PR was created.
There was a problem hiding this comment.
good catch, i made a mental shortcut, clarified in 3872fec
pr builds always use merge commit, not pr head
| uses: actions/checkout@v4 | ||
| with: | ||
| ref: ${{ github.event_name == 'pull_request_target' && github.event.pull_request.head.sha || github.sha }} | ||
| # No ref parameter needed - uses correct ref automatically: |
There was a problem hiding this comment.
This does change the behaviour to checkout the merge commit. I guess there's a reason for this being the default. So let's give it a go and change if needed in the future.
| uses: actions/checkout@v4 | ||
| with: | ||
| ref: ${{ github.event_name == 'pull_request_target' && github.event.pull_request.head.sha || github.sha }} | ||
| # No ref parameter needed - uses correct ref automatically: |
There was a problem hiding this comment.
The problem with this, which I just realised, is that this will cause ipfs-deploy-action to set the commit status for merge SHA commits that aren't visible in the pull request.
Both Cloudflare Pages and Vercel use the PR head commit for the following reasons:
- The preview deployment reflects exactly what's in your feature branch
- It doesn't include any potential merge conflicts or changes that would result from merging with the target branch
- Each new commit pushed to the PR branch triggers a new preview deployment from that latest commit
If we want to ensure that PRs are up to date and avoid merge conflicts, we can require branches to be up to date before merging in the branch protection rules in GitHub.
There was a problem hiding this comment.
I'm currently testing this in this PR: #2119
There was a problem hiding this comment.
For some reason, with #2119 the commit SHA that was used in the PR comment is pull_request.head.sha while the code that was checked out was technically the merge commit SHA (which should be different).
I'm currently investigating why.
There was a problem hiding this comment.
I can confirm now that there's a discrepancy between the commit SHA used for the build and the commit SHA used to update the comment.
You can see it here: https://github.com/ipfs/ipfs-docs/actions/runs/17208024892/job/48812841343#step:2:87
It's checking out and building e8daa37, but setting the commit status for a3bd938.
2 participants
splits workflow to enable ipfs deployments for all pull requests including those from forks
this approach is based on https://github.com/ipfs/specs where we conformed this approach works (PR from fork example: ipfs/specs#331 (comment))