Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[No QA] Fix checkDeploymentSuccess job in deploy workflow #49432

Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 14 additions & 8 deletions .github/workflows/deploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -455,34 +455,40 @@ jobs:
checkDeploymentSuccess:
runs-on: ubuntu-latest
outputs:
IS_AT_LEAST_ONE_PLATFORM_DEPLOYED: ${{ steps.checkDeploymentSuccess.outputs.IS_AT_LEAST_ONE_PLATFORM_DEPLOYED }}
IS_ALL_PLATFORMS_DEPLOYED: ${{ steps.checkDeploymentSuccess.outputs.IS_ALL_PLATFORMS_DEPLOYED }}
IS_AT_LEAST_ONE_PLATFORM_DEPLOYED: ${{ steps.checkDeploymentSuccessOnAtLeastOnePlatform.outputs.IS_AT_LEAST_ONE_PLATFORM_DEPLOYED }}
IS_ALL_PLATFORMS_DEPLOYED: ${{ steps.checkDeploymentSuccessOnAtLeastAllPlatform.outputs.IS_ALL_PLATFORMS_DEPLOYED }}
needs: [android, desktop, iOS, web]
if: ${{ always() }}
steps:
- name: Check deployment success on at least one platform
id: checkDeploymentSuccess
id: checkDeploymentSuccessOnAtLeastOnePlatform
run: |
isAtLeastOnePlatformDeployed="false"
isAllPlatformsDeployed="false"
if [ "${{ needs.android.result }}" == "success" ] || \
[ "${{ needs.iOS.result }}" == "success" ] || \
[ "${{ needs.desktop.result }}" == "success" ] || \
[ "${{ needs.web.result }}" == "success" ]; then
isAtLeastOnePlatformDeployed="true"
fi
echo "IS_AT_LEAST_ONE_PLATFORM_DEPLOYED=$isAtLeastOnePlatformDeployed" >> "$GITHUB_OUTPUT"
echo "IS_AT_LEAST_ONE_PLATFORM_DEPLOYED is $isAtLeastOnePlatformDeployed"

- name: Check deployment success on all platforms
id: checkDeploymentSuccessOnAtLeastAllPlatform
run: |
isAllPlatformsDeployed="false"
if [ "${{ needs.android.result }}" == "success" ] && \
[ "${{ needs.iOS.result }}" == "success" ] && \
[ "${{ needs.desktop.result }}" == "success" ] && \
[ "${{ needs.web.result }}" == "success" ]; then
isAllPlatformsDeployed="true"
fi
echo "IS_AT_LEAST_ONE_PLATFORM_DEPLOYED=\"$isAtLeastOnePlatformDeployed\"" >> "$GITHUB_OUTPUT"
echo "IS_ALL_PLATFORMS_DEPLOYED=\"$isAllPlatformsDeployed\"" >> "$GITHUB_OUTPUT"
echo "IS_ALL_PLATFORMS_DEPLOYED=$isAllPlatformsDeployed" >> "$GITHUB_OUTPUT"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this seems to be the main difference here; otherwise, splitting this up into two steps does not seem like much of a logical change, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the main change is removing the quotes to set the values as booleans, avoiding the truthy string issue. Splitting into two steps is just for clarity.

echo "IS_ALL_PLATFORMS_DEPLOYED is $isAllPlatformsDeployed"

createPrerelease:
runs-on: ubuntu-latest
if: ${{ github.ref == 'refs/heads/staging' && fromJSON(needs.checkDeploymentSuccess.outputs.IS_AT_LEAST_ONE_PLATFORM_DEPLOYED) }}
if: ${{ always() && github.ref == 'refs/heads/staging' && fromJSON(needs.checkDeploymentSuccess.outputs.IS_AT_LEAST_ONE_PLATFORM_DEPLOYED) }}
needs: [prep, checkDeploymentSuccess]
steps:
- name: Download all workflow run artifacts
Expand Down Expand Up @@ -540,7 +546,7 @@ jobs:

finalizeRelease:
runs-on: ubuntu-latest
if: ${{ github.ref == 'refs/heads/production' && fromJSON(needs.checkDeploymentSuccess.outputs.IS_AT_LEAST_ONE_PLATFORM_DEPLOYED) }}
if: ${{ always() && github.ref == 'refs/heads/production' && fromJSON(needs.checkDeploymentSuccess.outputs.IS_AT_LEAST_ONE_PLATFORM_DEPLOYED) }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you add the always here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The always() was added to ensure the condition is evaluated even if some platform deployments fail. Without it, these jobs could be skipped

needs: [prep, checkDeploymentSuccess]
steps:
- name: Download all workflow run artifacts
Expand Down
Loading