Failure notification on the daily sync and add persist-credentials: false and a permissions. - #74
Conversation
|
@coderabbitai, review |
|
@coderabbitai, fullreview |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughThe workflows now send Slack alerts for translation sync failures and stale heartbeat checks. A shared notification script builds workflow URLs, failed-job summaries, and Slack payloads. Tests mock Slack requests and validate notification behavior. Checkout permissions are restricted and persisted credentials are disabled. ChangesSlack alerting
Estimated code review effort: 3 (Moderate) | ~22 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/assets/notify.sh:
- Around line 52-60: Update send_slack_notification so its curl request includes
short connection and total request timeouts via --connect-timeout and
--max-time, while preserving the existing POST payload, headers, webhook URL,
and failure behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 499b07c4-6535-4869-91c2-3a5bad136b50
📒 Files selected for processing (11)
.github/workflows/add-submodules.yml.github/workflows/assets/notify.sh.github/workflows/heartbeat.yml.github/workflows/sync-translation.ymlCHANGELOG.mdREADME.mddocs/GETTING-STARTED.mdscripts/lint.shtests/helpers/common.bashtests/helpers/http_mock.bashtests/test_notify.bats
|
✅ Action performedReview finished.
|
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/GETTING-STARTED.md`:
- Around line 58-64: Update the workflow-specific requirements section in
GETTING-STARTED.md to state that sync-translation requires SYNC_TOKEN and
notify-failure requires SLACK_WEBHOOK_URL. Keep the existing descriptions and
formatting consistent with the surrounding documentation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4bb3b067-ff42-4030-9370-c22055b3f541
📒 Files selected for processing (11)
.github/workflows/add-submodules.yml.github/workflows/assets/notify.sh.github/workflows/heartbeat.yml.github/workflows/sync-translation.ymlCHANGELOG.mdREADME.mddocs/GETTING-STARTED.mdscripts/lint.shtests/helpers/common.bashtests/helpers/http_mock.bashtests/test_notify.bats
Close #66, close #67.
Summary by CodeRabbit
New Features
Documentation
Security
Tests