Update push and pr notification to use Cards v2. - #636
Conversation
brnnmrls
changed the title
w-goog
left a comment
w-goog
left a comment
There was a problem hiding this comment.
Approved assuming feedback is addressed :)
|
|
||
| curl --location --request POST "$WEBHOOK_URL" \ | ||
| --header 'Content-Type: application/json; charset=UTF-8' \ | ||
| --data "$PAYLOAD" |
There was a problem hiding this comment.
nit: Why the change to --data instead of --data-raw?
| }' | ||
| }') | ||
|
|
||
| curl --location --request POST "$WEBHOOK_URL" \ |
There was a problem hiding this comment.
I don't think we want this to fail a PR, but I do think we want it to indicate that something failed. For both curl commands, you could do all of: a) add --fail-with-body b) Capture the error output, and echo it in the log and c) add continue-on-error: true
| { | ||
| decoratedText: { | ||
| topLabel: "Labels", | ||
| text: (if $labels != "" then "- " + $labels else "None" end) |
There was a problem hiding this comment.
Please do a little helper function to add this guard on every text in both files - for example, I think it's unlikely that "github.event.head_commit.author.username" will be nil, but it is possible, which would cause this to fail. text is required by v2.
| - name: Google Chat Notification | ||
| shell: bash | ||
| env: | ||
| WEBHOOK_URL: ${{ secrets.WEBHOOK_URL }} |
There was a problem hiding this comment.
Since this will always be nil on PRs that come in from forks, can you add a quick check for WEBHOOK_URL that will skip the workflow if nil?
2 participants
This pull request updates .github/workflows/pr_notification.yml and .github/workflows/push_notification.yml to resolve an issue where notifications rendered as blank message bubbles in Google Chat. These files required updating the the following:
To fix this, both workflows have been migrated to modern Google Chat Cards v2 (cardsV2) utilizing decoratedText and buttonList widgets.