🚀 Publish previews on netlify#1088
Conversation
Signed-off-by: Alexandre Franke <alexandre.franke@matrix.org>
|
The second workflow will only be run if the file is in the default branch, so we can’t rely on this PR to check whether it works correctly. The changes to the build workflow have not broken it. @richvdh do you reckon I can merge this and fix it afterwards if it turns out not to work as expected? |
richvdh
left a comment
richvdh
left a comment
There was a problem hiding this comment.
do we not need to inhibit the netlify upload for non-PR builds?
| with: | ||
| script: | | ||
| var fs = require('fs'); | ||
| fs.writeFileSync('${{github.workspace}}/pr.json', JSON.stringify(context.payload.pull_request)); |
There was a problem hiding this comment.
how will this behave for builds not associated with a PR?
There was a problem hiding this comment.
It seems this will fail because prInfoArtifact.id is not defined (https://github.com/matrix-org/matrix-react-sdk/runs/3805077607?check_suite_focus=true would be an example of that).
We should probably handle this more gracefully.
There is no way to exit a job early, so that means that after the script step we would need to have an if for every subsequent step. Not great (but not necessarily an issue, just something to be aware of).
If on.workflow_run.branches-ignore does not exist and work, I reckon we have to run the script step in all cases as I don’t see a way to determine whether there was a PR before we try to download the PR metadata artifact.
There was a problem hiding this comment.
I misread the code this was referring to and thought we were talking about the script step of the netlify.yml workflow. Will read the documentation further.
| # There's a 'download artifact' action but it hasn't been updated for the | ||
| # workflow_run action (https://github.com/actions/download-artifact/issues/60) | ||
| # so instead we get this mess: |
There was a problem hiding this comment.
wouldn't it be easier just to put these steps in the other workflow?
There was a problem hiding this comment.
If we do that and for some reason netlify fails, then the build workflow status won’t be success.
There was a problem hiding this comment.
well yes, but how much is that really a problem?
| repo: context.repo.repo, | ||
| run_id: ${{github.event.workflow_run.id }}, | ||
| }); | ||
| var matchArtifact = artifacts.data.artifacts.filter((artifact) => { |
There was a problem hiding this comment.
there's something weird going on with the indentation here.
| var artifacts = await github.actions.listWorkflowRunArtifacts({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| run_id: ${{github.event.workflow_run.id }}, | ||
| }); | ||
| var matchArtifact = artifacts.data.artifacts.filter((artifact) => { | ||
| return artifact.name == "merged-content-artifact" | ||
| })[0]; | ||
| var download = await github.actions.downloadArtifact({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| artifact_id: matchArtifact.id, | ||
| archive_format: 'zip', | ||
| }); | ||
| var fs = require('fs'); | ||
| fs.writeFileSync('${{github.workspace}}/content.zip', Buffer.from(download.data)); | ||
|
|
||
| var prInfoArtifact = artifacts.data.artifacts.filter((artifact) => { | ||
| return artifact.name == "pr.json" | ||
| })[0]; | ||
| var download = await github.actions.downloadArtifact({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| artifact_id: prInfoArtifact.id, | ||
| archive_format: 'zip', | ||
| }); | ||
| var fs = require('fs'); | ||
| fs.writeFileSync('${{github.workspace}}/pr.json.zip', Buffer.from(download.data)); |
There was a problem hiding this comment.
if we really need this stuff.. maybe we should write our own action for it.
There was a problem hiding this comment.
Probably, although the ideal scenario really would be for the download-artifact action to be fixed, as pointed out by the comment. This comes from matrix-react-sdk BTW.
Can we consider this out of scope for this PR though?
There was a problem hiding this comment.
Probably, although the ideal scenario really would be for the download-artifact action to be fixed, as pointed out by the comment.
Sure, but given the issue has been open a year with no reply from the maintainers, let's not hold our breath on that one.
This comes from matrix-react-sdk BTW.
ok, so this is at least our second copy of it!
Can we consider this out of scope for this PR though?
I guess so. I do think we should do something else before we make a third copy.
My understanding is that once you've got the javascript, writing an action is pretty easy.
Signed-off-by: Alexandre Franke <alexandre.franke@matrix.org>
Signed-off-by: Alexandre Franke <alexandre.franke@matrix.org>
Signed-off-by: Alexandre Franke <alexandre.franke@matrix.org>
Signed-off-by: Alexandre Franke <alexandre.franke@matrix.org>
I would love to. I really can’t tell from the GHA documentation whether it’s possible though. There is no documented |
|
Considering the concerns that were raised here (and which I share), I’ll write another version of this that does everything in the initial workflow. I will do it on a different branch and open a new PR, and we can decide which one to run with once we have both. |
|
We went with #1089 instead. |
2 participants
This should publish a preview on netlify for every Actions run, with a link to it.