Skip to content
This repository has been archived by the owner on Apr 4, 2023. It is now read-only.

chore: added IN,NOT IN to invalid_filter msg #676

Merged
merged 2 commits into from Oct 31, 2022
Merged

chore: added IN,NOT IN to invalid_filter msg #676

merged 2 commits into from Oct 31, 2022

Conversation

Pranav-yadav
Copy link
Contributor

@Pranav-yadav Pranav-yadav commented Oct 31, 2022 •
edited

Pull Request

Related issue

Fixes meilisearch/meilisearch#3004

What does this PR do?

  • Improves correct error msg in response

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!

@Pranav-yadav Pranav-yadav mentioned this pull request Oct 31, 2022
Closed
3 tasks
@Pranav-yadav
Copy link
Contributor Author

This PR fixes meilisearch/meilisearch#3004

PS: do add hacktoberfest-accepted before merging :)

@curquiza
Copy link
Member

bors try

bors bot added a commit that referenced this pull request Oct 31, 2022
@bors
Try #676:
57134fa
@curquiza
Copy link
Member

@Pranav-yadav thanks for your PR

Can you remove the issue from the Development (on the right)? I don't want GitHub to close the issue on Meilisearch side when merging this PR 😇

@irevoire
Copy link
Member

Hey @Pranav-yadav, I think you forgot to update the tests.

@irevoire irevoire added the no breaking The related changes are not breaking (DB nor API) label Oct 31, 2022
@Pranav-yadav
Copy link
Contributor Author

Hey @Pranav-yadav, I think you forgot to update the tests.

Oops! On it..

@Pranav-yadav
Copy link
Contributor Author

Pranav-yadav commented Oct 31, 2022 •
edited

EDIT: Was able to understand some of code so updated them here 3950ec8

@irevoire can you guide me w/ fixing tests ?
I'm newbie in rust and it's stuff..

ran cargo test:
image

@Pranav-yadav
Copy link
Contributor Author

@curquiza Ohh! I though it's directly related issue hence mentioned..

@curquiza
Copy link
Member

@Pranav-yadav it's ok now! thanks 😇

@bors
Copy link
Contributor

bors bot commented Oct 31, 2022

try

Build succeeded:

@Pranav-yadav
Copy link
Contributor Author

Pranav-yadav commented Oct 31, 2022 •
edited

@irevoire can you guide me w/ fixing tests ? I'm newbie in rust and it's stuff..

ran cargo test: image

Did this much 3950ec8 , is it correct?

PS: Have exams tomorrow, and need to study hahahaha.. Signing off.. Will see this when I'm free, if any further changes required.

@irevoire
Copy link
Member

irevoire commented Oct 31, 2022 •
edited

Hey @Pranav-yadav, sorry that's an issue on our side, thanks for noticing I will open an issue for running the whole test suite in our CI and update our CONTRIBUTING.md maybe.

Did this much 3950ec8 , is it correct?

Yes this is perfect, thanks for your contribution!

bors merge

Since our CI had some issues recently I'm going to manually mark your PR as hacktoberfest-accepted, thus even if it doesn't merge your contribution will be marked as valid (in case you registered to participate to the hacktoberfest).
And in any case, if you want to receive some swag from Meilisearch, please complete this form, we only do it during the hacktoberfest 😁

irevoire
irevoire approved these changes Oct 31, 2022
View reviewed changes
Copy link
Member

@irevoire irevoire left a comment •
edited

Choose a reason for hiding this comment

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

thanks again!

@irevoire irevoire mentioned this pull request Oct 31, 2022
Merged
@bors
Copy link
Contributor

bors bot commented Oct 31, 2022

Build succeeded:

@bors bors bot merged commit 6770eb2 into meilisearch:main Oct 31, 2022
@meili-bot
Copy link
Contributor

This message is sent automatically

Thank you for contributing to Meilisearch. If you are participating in Hacktoberfest, and you would like to receive some gift from Meilisearch too, please complete this form.

bors bot added a commit that referenced this pull request Oct 31, 2022
@bors @irevoire
Merge #677
fe5a021 
677: run the tests in all workspaces r=curquiza a=irevoire

With #676 I noticed the tests were not running in any of our sub crates.
Most of our sub crates didn't includes any tests though.
But the filter-parser did and we're lucky we never broke these one without noticing 😁 

Co-authored-by: Irevoire <tamo@meilisearch.com>
@Pranav-yadav Pranav-yadav deleted the chore/improve-error-msg branch November 28, 2022 13:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@irevoire irevoire irevoire approved these changes

Assignees
No one assigned
Labels
no breaking The related changes are not breaking (DB nor API)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@Pranav-yadav @curquiza @irevoire @meili-bot