Implement restore-variable to clean our actions #239
Conversation
|
✅ Deployment SUCCESS |
|
✅ E2E tests SUCCESS for commit 19d11b0 previously deployed at https://nrn-v2-mst-aptd-at-lcz-sty-c1-gz3fryxf5.vercel.app |
|
✅ Deployment SUCCESS |
|
✅ E2E tests SUCCESS for commit ce5dec6 previously deployed at https://nrn-v2-mst-aptd-at-lcz-sty-c1-96zii3joa.vercel.app |
|
✅ Deployment SUCCESS |
|
✅ E2E tests SUCCESS for commit b41c89e previously deployed at https://nrn-v2-mst-aptd-at-lcz-sty-c1-3a7n8jpme.vercel.app |
|
✅ Deployment SUCCESS |
…gain pr_id_finder (remove one step)
| @@ -254,7 +230,7 @@ jobs: | |||
| env: | |||
| VERCEL_TOKEN: ${{ secrets.VERCEl_TOKEN }} | |||
| with: | |||
| deployment-url: ${{ env.VERCEL_DEPLOYMENT }} # Must only contain the domain name (no http prefix, etc.) | |||
| deployment-url: ${{ fromJson(steps.restore-variable.outputs.variables).VERCEL_DEPLOYMENT_DOMAIN }} # Must only contain the domain name (no http prefix, etc.) | |||
There was a problem hiding this comment.
@Demmonius Is there a better (shorter) way to do this?
Can't be variables automatically added to the ENV?
${{ env.VERCEL_DEPLOYMENT_DOMAIN }}
instead of
${{ fromJson(steps.restore-variable.outputs.variables).VERCEL_DEPLOYMENT_DOMAIN }}
It would simplify writing a lot.
There was a problem hiding this comment.
Something like https://github.com/marketplace/actions/add-env-vars might help
There was a problem hiding this comment.
Yes that's possible, I'll try to implement it asap
|
✅ E2E tests SUCCESS for commit e4a47bb previously deployed at https://nrn-v2-mst-aptd-at-lcz-sty-c1-9bjk35acb.vercel.app |
|
✅ Deployment SUCCESS |
|
|
||
| # Checking if a webhook url is defined | ||
| if [ -n "$VERCEL_DEPLOYMENT_COMPLETED_WEBHOOK" ]; then | ||
| if [ -n "$VERCEL_DEPLOYMENT_COMPLETED_WEBHOOK" ]; then |
There was a problem hiding this comment.
This might be outside this PR's scope, but if you know of any way to extract it as an external bash script, and call it as a package.json script, that'd be a great improvement for readability.
|
❌ E2E tests FAILED for commit 1a29dc7 previously deployed at https://nrn-v2-mst-aptd-at-lcz-sty-c1-bgq9cntdi.vercel.app |
|
✅ E2E tests SUCCESS for commit 1a29dc7 previously deployed at https://nrn-v2-mst-aptd-at-lcz-sty-c1-bgq9cntdi.vercel.app |
|
✅ Deployment SUCCESS |
| # On E2E failure, add a comment to the PR with additional information, if there is an open PR for the current branch | ||
| - name: Comment PR (E2E failure) | ||
| uses: peter-evans/create-or-update-comment@v1 # See https://github.com/peter-evans/create-or-update-comment | ||
| if: steps.pr_id_finder.outputs.number && failure() | ||
| if: fromJson(steps.restore-variable.outputs.variables).GITHUB_PULL_REQUEST_ID && success() |
There was a problem hiding this comment.
@Demmonius I've removed the step Finding Pull Request ID, but I'm not sure whether I should use fromJson(steps.restore-variable.outputs.variables).GITHUB_PULL_REQUEST_ID && success() or GITHUB_PULL_REQUEST_ID && success() here 🤔
Can you review?
There was a problem hiding this comment.
It looks like to be right. What are you thinking about? GITHUB_PULL_REQUEST_ID isn't declare right?
|
❌ E2E tests FAILED for commit 93e031b previously deployed at https://nrn-v2-mst-aptd-at-lcz-sty-c1-2v5sniwsi.vercel.app |
|
✅ E2E tests SUCCESS for commit 93e031b previously deployed at https://nrn-v2-mst-aptd-at-lcz-sty-c1-2v5sniwsi.vercel.app |
|
✅ Deployment SUCCESS |
|
❌ E2E tests FAILED for commit 6bf3567 previously deployed at https://nrn-v2-mst-aptd-at-lcz-sty-c1-lwbyqxpp3.vercel.app |
|
✅ E2E tests SUCCESS for commit 6bf3567 previously deployed at https://nrn-v2-mst-aptd-at-lcz-sty-c1-lwbyqxpp3.vercel.app |
Co-authored-by: Dhenain Ambroise <[email protected]>
Use https://github.com/UnlyEd/github-action-store-variable to share our variables across different jobs, thus reducing the code needed and avoiding code duplication as much as possible.
Also, it fixes an issue with E2E tests when deploying multiple commits in parallel, because we don't mistakenly await for the latest Vercel deployment, but await for the correct deployment instead.
Status
This PR only affects "staging" for now, we'll make another one that changes the "production" script.
Also, we'll change the github action implementation to restore variables as ENV variables for use of writing/DX. (later)