-
Notifications
You must be signed in to change notification settings - Fork 105
Migrate CI from bors to merge queue #801
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughUpdated CI workflow triggers, revised contributor documentation, and removed Bors configuration. Specifically: added a Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant GH as GitHub
participant CI as Actions Runner
Note over GH,CI #DDEBF7: Updated workflow triggers
Dev->>GH: push to `main` or `v1.x`
activate GH
GH->>CI: dispatch workflow (push)
deactivate GH
Dev->>GH: open PR (merge_group)
activate GH
GH->>CI: dispatch workflow (pull_request: merge_group)
deactivate GH
Note over CI #F7F6D9: Workflow jobs run (unchanged)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
💤 Files with no reviewable changes (2)
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.
Please see the documentation for more information. 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. 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: 0
🧹 Nitpick comments (1)
.github/workflows/tests.yml (1)
3-6
: Optional: Scope pull_request event types to cut CI noise.If you only need CI on code changes, consider restricting PR trigger types.
Apply this diff:
on: - pull_request: + pull_request: + types: [opened, synchronize, reopened, ready_for_review] merge_group:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/tests.yml
(1 hunks)
🔇 Additional comments (1)
.github/workflows/tests.yml (1)
3-6
: Merge queue trigger LGTM; confirm repo settings.Adding
merge_group
at the top level underon:
is correct for GitHub’s merge queue. Please confirm that merge queue is enabled (and protected) formain
andv1.x
, otherwise this event won’t fire.
f537241
to
dc36472
Compare
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: 0
🧹 Nitpick comments (1)
.github/workflows/tests.yml (1)
3-9
: Merge queue trigger added — add types and confirm branch protections.Good call adding
merge_group
. Per GitHub’s guidance, consider specifying activity types to future‑proof the trigger and make intent explicit.Apply:
on: pull_request: - merge_group: + merge_group: + types: [checks_requested] push: branches: - main - v1.x
- Rationale:
merge_group
is a separate event frompull_request
; GitHub recommends adding it explicitly and optionally scoping totypes: [checks_requested]
. (docs.github.com)- Please also verify your branch protection rules now require the same status checks for the merge queue (otherwise merges from the queue will stall). (docs.github.com)
Optional: Confirm that
v1.x
is an actual branch name (literal). If you intended a pattern for all v1 maintenance branches, usev1.*
instead. (No change needed ifv1.x
is the canonical branch.)
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.
i have no experience with it so just approving 🤷♂️
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.
I like the initiative! bors is a nightmare recently. I want to remove it everywhere for merge queue
Can you remove all bors mentions?
- bors toml file
- mention in contributing
- ...
e1e5b57
to
621e53f
Compare
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: 0
🧹 Nitpick comments (3)
CONTRIBUTING.md (3)
3-3
: Intro copy reads well; optional tightening.
Consider dropping “in order to” for brevity.Apply if you prefer tighter prose:
-Thank you for considering contributing to Meilisearch! This document provides everything you need to know in order to contribute to Meilisearch PHP. +Thank you for considering contributing to Meilisearch! This document provides everything you need to know to contribute to Meilisearch PHP.
28-31
: PR submission guidance: OK; minor clarity tweak.
Suggest explicitly mentioning “from your fork” for first‑timers.-[Submit the branch as a PR](https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/creating-a-pull-request-from-a-fork) pointing to the `main` branch of the main meilisearch-php repository. +[From your fork, submit the branch as a PR](https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/creating-a-pull-request-from-a-fork) pointing to the `main` branch of the meilisearch-php repository.
86-86
: Merge Queue note is correct; add how to update branch.
Call out rebase/merge instructions to resolve “update required” notices before queueing.-- The branch related to the PR must be **up-to-date with `main`** before merging. This project uses [Merge Queues](https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/configuring-pull-request-merges/managing-a-merge-queue) to automatically enforce this requirement. +- The branch related to the PR must be **up-to-date with `main`** before merging. This project uses [Merge Queues](https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/configuring-pull-request-merges/managing-a-merge-queue) to enforce this. + If the PR shows “update required,” rebase or merge `main` into your branch, then re-queue.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/tests.yml
(1 hunks)CONTRIBUTING.md
(5 hunks)bors.toml
(0 hunks)
💤 Files with no reviewable changes (1)
- bors.toml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/tests.yml
🔇 Additional comments (6)
CONTRIBUTING.md (6)
96-96
: Release Drafter mention: good alignment with CI changes.
No issues.
106-110
: Version constant snippet: OK.
Matches typical pattern for PHP SDKs.
112-114
: Possible typo in external anchor; please confirm link works.
The anchor path contains “writting-the-release-description”. If the upstream doc uses “writing…”, the link may be off.If needed, update:
-.../integration-release.md#writting-the-release-description +.../integration-release.md#writing-the-release-descriptionAlso verify Release page link and Packagist coordinates are correct.
118-118
: Friendly closing: nice touch.
Good contributor experience.
102-103
: Version bump workflow target verified
src/Meilisearch.php exists and declarespublic const VERSION = '1.16.1'
;.github/workflows/update-version.yml
is present — workflow can update that constant.
17-19
: Verify external links — two redirects found, one OKCONTRIBUTING.md (lines 17–19): README link returns 200; GitHub PR help URL returns 301; dub.sh returns 302. Follow redirects (curl -L) and confirm final destinations return 200, or replace the redirected URLs with their canonical targets (docs.github.com and the final Discord invite).
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.gitattributes
(0 hunks).github/release-draft-template.yml
(1 hunks)README.md
(0 hunks)
💤 Files with no reviewable changes (2)
- README.md
- .gitattributes
Pull Request
what
@curquiza recommends enabling merge queues on all Meilisearch repositories to replace using
bors
.how
merge_group
trigger to the test workflowPR checklist
Please check if your PR fulfills the following requirements:
Thank you so much for contributing to Meilisearch!
Summary by CodeRabbit
Chores
Documentation