-
Notifications
You must be signed in to change notification settings - Fork 225
[CI] Merge main to develop after release on the same workflow run #3808
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
Conversation
WalkthroughRemoves the standalone merge-main-to-develop workflow, adds the same merge step as a dependent job in release-publish, renames a job in release-merge, and deletes the Fastlane trigger that invoked the removed workflow. Slack failure notifications are preserved within the new job path. No application/runtime code changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant GH as GitHub Actions
participant RL as release (job)
participant MM as merge-main-to-develop (job)
participant FL as Fastlane
participant SL as Slack
Dev->>GH: Push tag / trigger release workflow
GH->>RL: Run release job
RL-->>GH: Success
GH->>MM: Start dependent merge job
MM->>GH: actions/checkout (ADMIN_API_TOKEN)
MM->>FL: bundle exec fastlane merge_main (GITHUB_TOKEN=ADMIN_API_TOKEN)
alt merge failure
MM-->>SL: Post failure notification
else merge success
MM-->>GH: Complete
end
sequenceDiagram
autonumber
actor Dev as Developer
participant OLD as (Old) Separate Workflow
participant NEW as (New) release-publish.yml
Note over OLD: Previously
Dev->>OLD: workflow_dispatch\n"merge-main-to-develop.yml"
OLD->>OLD: checkout → ruby-cache → fastlane merge_main
alt failure
OLD->>OLD: Slack notify
end
Note over NEW: Now
Dev->>NEW: release job completes
NEW->>NEW: chained job "merge-main-to-develop"\ncheckout → ruby-cache → fastlane merge_main
alt failure
NEW->>NEW: Slack notify
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (5 passed)✅ Passed checks (5 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal). Please share your feedback with us on this Discord post. ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 1
🧹 Nitpick comments (8)
.github/workflows/release-merge.yml (3)
12-14
: Guard against unintended triggers; add timeout and concurrency.
- Add a timeout to avoid hanging runs.
- Add a concurrency group so multiple “/merge release” comments don’t overlap.
Apply:
merge-release-to-main: name: Merge release to main runs-on: macos-15 + timeout-minutes: 30 + concurrency: + group: release-merge-${{ github.ref }} + cancel-in-progress: false
15-21
: Pin actions to commit SHAs.
Reduce supply-chain risk by pinning webfactory/ssh-agent and actions/checkout to specific commit SHAs rather than tags.If you want, I can look up the latest recommended SHAs and propose exact replacements.
24-31
: Restrict default GITHUB_TOKEN permissions.
Since pushes/auth are via ADMIN_API_TOKEN/SSH, explicitly set minimal permissions to harden the workflow.Apply (at workflow or job level):
jobs: + permissions: + contents: read merge-release-to-main:.github/workflows/release-publish.yml (5)
35-41
: Harden checkout and caching.
- Pin actions/checkout to a commit SHA.
- Ensure ./.github/actions/ruby-cache supports Ubuntu; if it’s macOS-specific, switch to ruby/setup-ruby with bundler-cache.
I can suggest a ruby/setup-ruby snippet if needed.
46-53
: Include artifacts for debugging failed merges.
Upload Fastlane logs on failure to speed up triage.Apply:
- uses: 8398a7/action-slack@v3 if: failure() with: status: ${{ job.status }} text: "⚠️ <!subteam^S030AAHLDLN>, the merge of `main` to `develop` failed on CI. Consider using this command locally: `bundle exec fastlane merge_main`" fields: repo,commit,author,workflow env: SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL }} + + - name: Upload merge logs + if: failure() + uses: actions/upload-artifact@v4 + with: + name: merge-main-logs + path: | + fastlane/logs + fastlane/report.xml + if-no-files-found: ignore
30-33
: Add timeout and concurrency to avoid overlapping merges.Apply:
merge-main-to-develop: name: Merge main to develop - runs-on: ubuntu-latest + runs-on: ubuntu-latest + timeout-minutes: 20 + concurrency: + group: merge-main-to-develop-${{ github.ref }} + cancel-in-progress: false
30-54
: Set minimal permissions for the workflow.Apply (top-level or per job):
jobs: + permissions: + contents: read release:If release or merge needs more, scope them per-job explicitly.
35-39
: Minor: ensure checkout uses the intended credentials.
You pass token: ADMIN_API_TOKEN (good). Consider persist-credentials: false to avoid accidental fallback to GITHUB_TOKEN from other actions.Apply:
- uses: actions/[email protected] with: token: ${{ secrets.ADMIN_API_TOKEN }} fetch-depth: 0 + persist-credentials: false
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
.github/workflows/merge-main-to-develop.yml
(0 hunks).github/workflows/release-merge.yml
(1 hunks).github/workflows/release-publish.yml
(1 hunks)fastlane/Fastfile
(0 hunks)
💤 Files with no reviewable changes (2)
- fastlane/Fastfile
- .github/workflows/merge-main-to-develop.yml
🔇 Additional comments (5)
.github/workflows/release-merge.yml (3)
10-31
: Job rename is fine; behavior unchanged.
The identifier change to merge-release-to-main keeps the logic/steps intact. No functional concerns spotted.
12-12
: Verify macOS runner availability.
Confirm macos-15 is available to your org and not deprecated. If not guaranteed, consider macos-latest with a scheduled audit.
27-31
: Membership/authorization check lives in Fastlane—confirm it’s enforced.
Given anyone could comment “/merge release” on public PRs, ensure merge_release validates the commenter/org membership using ADMIN_API_TOKEN (team lookup). Looks implied; please confirm..github/workflows/release-publish.yml (2)
30-54
: Good move embedding the main→develop merge into the release pipeline.
Sequencing via needs: release and failure-only Slack alert look solid.
32-33
: Run on Linux only if the lane is macOS-agnostic.
merge_main should be fine on Ubuntu (git-only), but verify the Fastlane lane doesn’t require Xcode/macOS gems.
- run: bundle exec fastlane merge_main | ||
env: | ||
GITHUB_TOKEN: ${{ secrets.ADMIN_API_TOKEN }} | ||
|
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.
🛠️ Refactor suggestion
Add retry/backoff to mitigate transient GitHub outages (root problem of IOS-1116).
Wrap merge_main with bounded retries and exponential backoff.
Apply:
- - run: bundle exec fastlane merge_main
- env:
- GITHUB_TOKEN: ${{ secrets.ADMIN_API_TOKEN }}
+ - name: Merge main to develop (with retries)
+ env:
+ GITHUB_TOKEN: ${{ secrets.ADMIN_API_TOKEN }}
+ run: |
+ set -euo pipefail
+ for i in 1 2 3; do
+ echo "Attempt $i..."
+ if bundle exec fastlane merge_main; then
+ exit 0
+ fi
+ sleep $((i * 30))
+ done
+ echo "All attempts failed"
+ exit 1
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
- run: bundle exec fastlane merge_main | |
env: | |
GITHUB_TOKEN: ${{ secrets.ADMIN_API_TOKEN }} | |
- name: Merge main to develop (with retries) | |
env: | |
GITHUB_TOKEN: ${{ secrets.ADMIN_API_TOKEN }} | |
run: | | |
set -euo pipefail | |
for i in 1 2 3; do | |
echo "Attempt $i..." | |
if bundle exec fastlane merge_main; then | |
exit 0 | |
fi | |
sleep $((i * 30)) | |
done | |
echo "All attempts failed" | |
exit 1 |
🔗 Issue Links
Copy-paste GetStream/stream-video-swift#930
Resolve https://linear.app/stream/issue/IOS-1116
🎯 Goal
Minimize the risk of missing merge back to develop.
Summary by CodeRabbit