Skip to content

Conversation

testableapple
Copy link
Contributor

@testableapple testableapple commented Sep 9, 2025

🔗 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

  • Chores
    • Consolidated the post-release “main to develop” merge into the release pipeline, including failure notifications.
    • Removed the standalone workflow that previously handled the “main to develop” merge.
    • Standardized job naming within the release workflow.
    • No changes to application functionality, user interface, or public APIs.

@testableapple testableapple requested a review from a team as a code owner September 9, 2025 14:48
@testableapple testableapple added the 🤖 CI/CD Any work related to CI/CD label Sep 9, 2025
Copy link

coderabbitai bot commented Sep 9, 2025

Walkthrough

Removes 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

Cohort / File(s) Summary
Remove standalone merge workflow
.github/workflows/merge-main-to-develop.yml
Deleted the “Merge main to develop” workflow (checkout, Ruby cache, fastlane merge_main, Slack on failure).
Rename job in release merge workflow
.github/workflows/release-merge.yml
Renamed job id from merge-comment to merge-release-to-main; no functional step changes.
Embed merge into release publish workflow
.github/workflows/release-publish.yml
Added job merge-main-to-develop after release: checkout with ADMIN_API_TOKEN, Ruby cache, run bundle exec fastlane merge_main, Slack notify on failure.
Remove Fastlane workflow trigger
fastlane/Fastfile
Removed gh workflow run merge-main-to-develop.yml --ref main from publish_release; leaves update_spm(version: release_version).

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
Loading
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
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • nuno-vieira
  • martinmitrevski

Pre-merge checks (5 passed)

✅ Passed checks (5 passed)
Check name Status Explanation
Title Check ✅ Passed The title “[CI] Merge main to develop after release on the same workflow run” succinctly and accurately describes the primary change of integrating the merge step into the release workflow run without extraneous detail, making it clear to reviewers what the central CI update entails.
Linked Issues Check ✅ Passed The changes add a “merge-main-to-develop” job in the release-publish workflow with Slack notifications on failure, directly addressing IOS-1116’s requirement to track the progress of the merge main to develop action and detect failures.
Out of Scope Changes Check ✅ Passed All modifications—including removing the standalone workflow, renaming the merge job identifier, adding the merge step to the release workflow, and updating Fastfile triggers—are focused on the CI merge process and align with the linked issue’s objectives without introducing unrelated changes.
Description Check ✅ Passed The description clearly links to the relevant issue and succinctly states the goal of minimizing missed merges back to develop, which directly relates to the CI workflow changes introduced in this pull request.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

Poem

Thump-thump go my paws in CI land,
We moved the merge to follow the band.
No stray workflows hop astray,
Slack will squeak if things delay.
Carrots queued when merges pass—
Ship the release, then quick as grass!
🥕✨

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.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

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
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch ci/merge-main

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 0041aa2 and f74b706.

📒 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.

Comment on lines +42 to +45
- run: bundle exec fastlane merge_main
env:
GITHUB_TOKEN: ${{ secrets.ADMIN_API_TOKEN }}

Copy link

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.

Suggested change
- 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

@testableapple testableapple merged commit 77ba7f0 into develop Sep 9, 2025
0 of 6 checks passed
@testableapple testableapple deleted the ci/merge-main branch September 9, 2025 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖 CI/CD Any work related to CI/CD
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant