Skip to content

ping Slack #tabs-vs-spaces when E2E tests fail - #269

Merged
NejcS merged 1 commit into
mainfrom
worktree-e2e-slack-notify
Aug 17, 2026
Merged

ping Slack #tabs-vs-spaces when E2E tests fail#269
NejcS merged 1 commit into
mainfrom
worktree-e2e-slack-notify

Conversation

@NejcS

@NejcS NejcS commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Adds a notify-on-failure job to e2e.yml that posts to Slack #tabs-vs-spaces whenever the Linux or Windows E2E job fails. Guarded on the new SLACK_E2E_WEBHOOK_URL secret (couldn't reuse the other one since it's for the wrong channel), so repos without it are unaffected. Follows the curl pattern from #268.

@NejcS
NejcS requested a review from hisenb3rg July 31, 2026 08:59
@NejcS
NejcS marked this pull request as ready for review July 31, 2026 09:00
@NejcS NejcS changed the title ci: ping Slack #tabs-vs-spaces when E2E tests fail ping Slack #tabs-vs-spaces when E2E tests fail Jul 31, 2026

@hisenb3rg hisenb3rg left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yo,

I've asked my agent and brought some suggestions up, I think they make sense:

  • Repo already has the safe pattern — reuse it. nofity-slack-on-main-merge.yml:15-22 uses slackapi/slack-github-action@v1.24.0 toJSON(format(...)). That gets you correct escaping, no injection, and non-2xx → job failure, for free. Hand-rolled curl is a regression from your own convention.
  • Message doesn't say which platform failed. Two jobs in needs; you get pinged and still have to open the run. Add needs.*.result.
  • Missing newline at EOF. Commit fixed the old one and introduced a new one.

@NejcS
NejcS requested a review from hisenb3rg August 13, 2026 13:18
@NejcS

NejcS commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

All 3 make sense, thanks.

@hisenb3rg hisenb3rg left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM 👍

@NejcS
NejcS force-pushed the worktree-e2e-slack-notify branch from db65b02 to 35ffb68 Compare August 17, 2026 09:17
@NejcS
NejcS merged commit a61f500 into main Aug 17, 2026
10 checks passed
@NejcS
NejcS deleted the worktree-e2e-slack-notify branch August 17, 2026 09:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants