Skip to content

Conversation

@norkunas
Copy link
Collaborator

@norkunas norkunas commented Nov 29, 2025

Pull Request

Related issue

Fixes #<issue_number>

What does this PR do?

  • Adds PHP 8.5 to CI

PR checklist

Please check if your PR fulfills the following requirements:

  • Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
  • Have you read the contributing guidelines?
  • Have you made sure that the title is accurate and descriptive of the changes?

Summary by CodeRabbit

  • Tests
    • Added PHP 8.5 to test coverage across multiple CI workflows for broader compatibility verification.
  • Refactor
    • Several exception classes now implement the Stringable interface, tightening type contracts without changing runtime behavior.

✏️ Tip: You can customize this high-level summary in your review settings.

@norkunas norkunas requested a review from Strift November 29, 2025 20:45
@norkunas norkunas added enhancement New feature or request maintenance Anything related to maintenance (CI, tests, refactoring...) labels Nov 29, 2025
@coderabbitai
Copy link

coderabbitai bot commented Nov 29, 2025

Walkthrough

Adds PHP 8.5 to three GitHub Actions workflows' PHP matrices and runtime references, and updates four exception classes to implement the \Stringable interface in addition to ExceptionInterface.

Changes

Cohort / File(s) Change Summary
GitHub Actions Workflows — PHP 8.5 Addition
.github/workflows/meilisearch-beta-tests.yml, .github/workflows/pre-release-tests.yml, .github/workflows/tests.yml
Extended PHP test matrices and updated runtime version references to include PHP 8.5 alongside existing versions. No other workflow logic changed.
Exceptions — Add \Stringable
src/Exceptions/ApiException.php, src/Exceptions/CommunicationException.php, src/Exceptions/InvalidResponseBodyException.php, src/Exceptions/TimeOutException.php
Each exception class declaration updated to implement \Stringable in addition to ExceptionInterface. No behavioral or method changes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Mixed changes: CI configuration updates and public API surface change (added interface on multiple exception classes).
  • Review focus:
    • Confirm adding \Stringable to exceptions is intended and consistent with project public contracts.
    • Ensure no downstream code relies on exact implemented interfaces or causes signature/interface conflicts.
    • Quick check of workflows to validate syntax and matrix consistency.

Suggested labels

CI

Suggested reviewers

  • brunoocasali

Poem

🐰
I hopped through code with eager paws,
Added eight-dot-five to CI laws.
Four exceptions now speak in string,
Tests grow wider — let the runners sing! 🎉

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: adding PHP 8.5 to CI workflows across multiple files (.github/workflows/*.yml) and exception classes.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5357e98 and a37f95d.

📒 Files selected for processing (4)
  • src/Exceptions/ApiException.php (1 hunks)
  • src/Exceptions/CommunicationException.php (1 hunks)
  • src/Exceptions/InvalidResponseBodyException.php (1 hunks)
  • src/Exceptions/TimeOutException.php (1 hunks)
🔇 Additional comments (4)
src/Exceptions/CommunicationException.php (1)

7-12: Explicit \Stringable implementation here looks correct

This class already defines __toString(): string, so adding implements \Stringable simply formalizes the contract and is fully compatible with the package’s PHP ^8.1+ requirement and PHP’s recommendation to declare Stringable explicitly when __toString is present. (packagist.org)

Please just confirm that your CI matrix and composer.json’s PHP constraint stay in sync (including the new PHP 8.5 target).

src/Exceptions/TimeOutException.php (1)

7-28: TimeOutException now correctly advertises its stringability

Given the existing __toString(): string implementation, declaring implements \Stringable is contract-safe and just makes the string representation available to type checks (e.g. string|\Stringable) without changing behavior.

If you have any union type-hints elsewhere that expect these exceptions as stringable inputs, consider updating them to string|\Stringable to take advantage of this.

src/Exceptions/ApiException.php (1)

9-53: ApiException’s new \Stringable contract matches the existing implementation

The class already exposes a detailed __toString(): string representation, so adding implements \Stringable is purely declarative and safe, enabling stricter typing without altering runtime behavior.

Consider scanning for any consumers that could now type-hint against \Stringable instead of a looser type when working with this exception.

src/Exceptions/InvalidResponseBodyException.php (1)

9-47: InvalidResponseBodyException now cleanly participates in \Stringable typing

The existing __toString(): string already fulfills the \Stringable contract, so adding the interface is a non-breaking enhancement that makes this exception usable wherever a Stringable is expected.

If you later introduce stricter type declarations (e.g., mixed $httpBody in the constructor), ensure they remain compatible with any external usages before changing the signature.


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.

@codecov
Copy link

codecov bot commented Nov 29, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.02%. Comparing base (0078a8c) to head (a37f95d).
⚠️ Report is 92 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #829      +/-   ##
==========================================
- Coverage   89.78%   88.02%   -1.76%     
==========================================
  Files          59       81      +22     
  Lines        1449     1729     +280     
==========================================
+ Hits         1301     1522     +221     
- Misses        148      207      +59     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Strift Strift added this pull request to the merge queue Dec 1, 2025
Merged via the queue into meilisearch:main with commit 565bca2 Dec 1, 2025
11 of 12 checks passed
@norkunas norkunas deleted the php85 branch December 1, 2025 04:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request maintenance Anything related to maintenance (CI, tests, refactoring...)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants