Migrate slack notifications to composite action#138
Conversation
- Replace standalone slack-notification jobs with inline composite action step - Pass SLACK_BOT_TOKEN and SLACK_CHANNEL to reusable workflows that now handle notifications internally - Add concurrency groups to workflows missing them (skip tag-only and project management workflows) - Remove unused job outputs (status) that were only needed for the old notification pattern - Use sdk-versions composite action instead of hardcoded version lists (code-snippets-v4)
🤖 Claude Code ReviewPR Code ReviewSummaryThis PR migrates Slack failure notifications from a separate reusable workflow job ( Code QualityStyle guide ✅ No commented-out code ✅ Meaningful variable names ✅ DRY principle ✅ Defects ❌ Critical behavior regression in - name: Notify Slack on failure
if: (failure() || cancelled())In GitHub Actions, when a job is cancelled, pending steps are set to The old pattern correctly used if: ${{ always() && contains(fromJSON('["failure", "cancelled"]'), needs.docker-push-containers-to-dockerhub.result) }}Fix: if: always() && (failure() || cancelled())Minor concern in Testing✅ N/A — workflow YAML changes; no unit/integration tests required. DocumentationCHANGELOG.md — The README / API docs ✅ — Not applicable for workflow-only changes. SecurityNo hardcoded credentials ✅ Secrets handling ✅ — All secrets are referenced via zizmor suppression ✅ — Adding No license files ( Summary Table
Action RequiredThe
|
Super-linter summary
Super-linter detected linting errors For more information, see the GitHub Actions workflow run Powered by Super-linter GITHUB_ACTIONS_ZIZMOR |
🤖 Claude Code ReviewCode ReviewThis PR refactors GitHub Actions workflows to consolidate Slack notifications, add concurrency controls, and update triggers. Here is the review: Code Quality✅ No commented-out code — All removed lines are active YAML, not comments. ✅ Meaningful variable names — Workflow names, job IDs, and secret references are clear and consistent. ✅ DRY principle — The PR reduces repetition by consolidating Slack notification logic: either delegating it to called reusable workflows (passing ✅ No defects detected — However, two behavioral changes are worth confirming are intentional:
✅ No Testing✅ N/A — Changes are limited to CI/CD workflow YAML files. No application logic was modified. Documentation✅ README not required — These are internal CI workflow changes with no user-facing API impact.
✅ No complex inline logic — The YAML changes are straightforward; no additional comments are needed. ✅ Markdown formatting — The YAML files are well-structured. No Markdown formatting issues in the diff. Security✅ No hardcoded credentials — All secrets use ✅ ✅ No sensitive data in logs — No ✅ No ✅ Input validation N/A — No application logic modified. SummaryThe PR is clean and the changes are well-scoped. Two items to confirm before merging:
Automated code review analyzing defects and coding standards |
Super-linter summary
All files and directories linted successfully For more information, see the GitHub Actions workflow run Powered by Super-linter |
2 participants
Summary
slack-notificationjobs with inline composite action step fromsenzing-factory/build-resources/slack-failure-notification@v4SLACK_BOT_TOKENandSLACK_CHANNELsecrets to reusable workflows (add-labels-to-issue,add-to-project,add-to-project-dependabot) that now handle notifications internallyoutputs: statusfrom jobs that only existed for the old notification patternsdk-versionscomposite action instead of hardcoded version lists (code-snippets-v4 only)Test plan