Skip to content
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

feat: Adds hybrid search options contract for multisearch #677

Merged
merged 7 commits into from
Sep 15, 2024

Conversation

apozeus
Copy link
Contributor

@apozeus apozeus commented Sep 9, 2024

Pull Request

Related issue

Fixes #676

What does this PR do?

Adds hybrid search options contract for multisearch

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?

Thank you so much for contributing to Meilisearch!

@norkunas
Copy link
Collaborator

norkunas commented Sep 9, 2024

@apozeus why you opened same code changes with two separate pull requests? 🙂

@apozeus
Copy link
Contributor Author

apozeus commented Sep 9, 2024

@apozeus why you opened same code changes with two separate pull requests? 🙂

Oh my mistake. i thought i needed to open an issue then make a PR.

@norkunas
Copy link
Collaborator

norkunas commented Sep 9, 2024

@apozeus why you opened same code changes with two separate pull requests? 🙂

Oh my mistake. i thought i needed to open an issue then make a PR.

Well you could open an issue and then link in the previous PR description 🙂
As this is I think a priority bug/feature and easy to fix it, at least I'd merge also without opening the issue.

By the way, I've left some comments in your previous PR.

@apozeus
Copy link
Contributor Author

apozeus commented Sep 9, 2024

@apozeus why you opened same code changes with two separate pull requests? 🙂

Oh my mistake. i thought i needed to open an issue then make a PR.

Well you could open an issue and then link in the previous PR description 🙂 As this is I think a priority bug/feature and easy to fix it, at least I'd merge also without opening the issue.

By the way, I've left some comments in your previous PR.

Commited your comments.

@norkunas
Copy link
Collaborator

Ok so as you left this one open, then please apply all code change suggestions from previous PR

@apozeus
Copy link
Contributor Author

apozeus commented Sep 10, 2024

Ok so as you left this one open, then please apply all code change suggestions from previous PR

it's all applied.

@norkunas
Copy link
Collaborator

Ok so as you left this one open, then please apply all code change suggestions from previous PR

it's all applied.

Nope :) I don't see it, maybe you forgot to commit?

Copy link

codecov bot commented Sep 11, 2024

Codecov Report

Attention: Patch coverage is 6.66667% with 14 lines in your changes missing coverage. Please review.

Project coverage is 82.47%. Comparing base (ccd4cfd) to head (f9432dd).
Report is 33 commits behind head on main.

Files with missing lines Patch % Lines
src/Contracts/HybridSearchOptions.php 0.00% 11 Missing ⚠️
src/Contracts/SearchQuery.php 25.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #677      +/-   ##
==========================================
- Coverage   83.91%   82.47%   -1.45%     
==========================================
  Files          49       52       +3     
  Lines        1262     1318      +56     
==========================================
+ Hits         1059     1087      +28     
- Misses        203      231      +28     

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

@brunoocasali
Copy link
Member

bors merge

@meili-bors meili-bors bot merged commit ae4f430 into meilisearch:main Sep 15, 2024
27 of 30 checks passed
@brunoocasali brunoocasali added the enhancement New feature or request label Sep 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hybrid search options missing for new federated search feature in v1.10 Meilisearch
3 participants