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

Add AI-powered search changes related to v1.10 #593

Merged
merged 2 commits into from
Dec 30, 2024

Conversation

Ja7ad
Copy link
Collaborator

@Ja7ad Ja7ad commented Dec 23, 2024

Pull Request

Related issue

Fixes #576

What does this PR do?

  • Update the embedders types + tests according to v1.10
  • Fix name convation for "APIKey" instead "ApiKey".

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!

@Ja7ad Ja7ad requested a review from curquiza December 23, 2024 06:47
Copy link

codecov bot commented Dec 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.22%. Comparing base (c167439) to head (13a25b1).
Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #593   +/-   ##
=======================================
  Coverage   86.22%   86.22%           
=======================================
  Files          14       14           
  Lines        2845     2845           
=======================================
  Hits         2453     2453           
  Misses        282      282           
  Partials      110      110           

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

@Ja7ad Ja7ad requested a review from brunoocasali December 23, 2024 07:59
@curquiza
Copy link
Member

@Ja7ad is this breaking?
If yes, can you list the breaking changes for the users (in PR description)?
To be clear, I have not issue with breaking of course, we just need clarity.

@curquiza curquiza added the enhancement New feature or request label Dec 30, 2024
@Ja7ad
Copy link
Collaborator Author

Ja7ad commented Dec 30, 2024

@Ja7ad is this breaking?
If yes, can you list the breaking changes for the users (in PR description)?
To be clear, I have not issue with breaking of course, we just need clarity.

Based on issue, it's breaking changes.

@curquiza
Copy link
Member

curquiza commented Dec 30, 2024

I asked because I see you also changed the name of Apikey into APIKey, and it's not part of the issue. Is there any other changes?

@Ja7ad
Copy link
Collaborator Author

Ja7ad commented Dec 30, 2024

I asked because I see you also changed the name of Apikey into APIKey, and it's not part of the issue. Is there any other changes?

Revert it?

@curquiza
Copy link
Member

@Ja7ad I'm not sure what you mean here
I want to know what are the breaking changes for users in this PR
Of course, there is the changes asked in the issues. But is there any other changes? I see in the PRs you also took the opportunity to change naming that are not asked in the issue. So now I'm asking if there is another other changes like this in the PR that are breaking for users? I need this information to deliver a clear changelogs

@Ja7ad
Copy link
Collaborator Author

Ja7ad commented Dec 30, 2024

@Ja7ad I'm not sure what you mean here
I want to know what are the breaking changes for users in this PR
Of course, there is the changes asked in the issues. But is there any other changes? I see in the PRs you also took the opportunity to change naming that are not asked in the issue. So now I'm asking if there is another other changes like this in the PR that are breaking for users? I need this information to deliver a clear changelogs

I mean do you want revert name of "ApiKey"?

@curquiza
Copy link
Member

I mean do you want revert name of "ApiKey"?

No I don't want, I trust you on what are the best changes.
But I need you to be clear in the PR description about the changes you made, so that I can document it clearly in the changelogs

@Ja7ad
Copy link
Collaborator Author

Ja7ad commented Dec 30, 2024

I mean do you want revert name of "ApiKey"?

No I don't want, I trust you on what are the best changes.
But I need you to be clear in the PR description about the changes you made, so that I can document it clearly in the changelogs

The word API is an abbreviation of three words and, in terms of naming convention, is better written as "API" instead of "Api".

Copy link
Member

@curquiza curquiza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bors merge

Copy link
Contributor

meili-bors bot commented Dec 30, 2024

@meili-bors meili-bors bot merged commit add5268 into meilisearch:main Dec 30, 2024
6 checks passed
@curquiza curquiza added breaking-change The related changes are breaking for the users and removed enhancement New feature or request labels Dec 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change The related changes are breaking for the users
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[v1.10.0] AI-powered search changes
2 participants