-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
gha(update-docs-webhook): fix workflow syntax #50778
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Would prefer consistent syntax with other workflows (unless there is some advantage to this approach) but this is really minor.
@@ -18,7 +18,7 @@ jobs: | |||
steps: | |||
- name: Call deployment webhook | |||
env: | |||
WEBHOOK_URL: ${{ secrets[AMPLIFY_DOCS_DEPLOY_HOOK] }} | |||
WEBHOOK_URL: ${{ secrets["AMPLIFY_DOCS_DEPLOY_HOOK"] }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Why use this syntax instead of the secrets.AMPLIFY_DOCS_DEPLOY_HOOK
used everywhere else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was done this way to support matrix, when we had to call two webhooks:
teleport/.github/workflows/update-docs-webhook.yaml
Lines 18 to 31 in b84f071
matrix: webhooks: - url_secret_name: DOCS_DEPLOY_HOOK http_method: GET - url_secret_name: AMPLIFY_DOCS_DEPLOY_HOOK http_method: POST steps: - name: Call deployment webhook env: WEBHOOK_URL: ${{ secrets[matrix.webhooks.url_secret_name] }} run: | if curl -X ${{ matrix.webhooks.http_method }} --silent --fail --show-error "$WEBHOOK_URL" > /dev/null; then echo "Triggered successfully" fi
Now when second webhook and matrix is removed, there is no real reason to keep it this way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in a9b7765
Summary
Workflow syntax introduced in:
Currently this action is failing on some of the branches where backports were merged: