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

Geosearch for zero radius #722

Merged
merged 2 commits into from
Dec 5, 2022
Merged

Conversation

amab8901
Copy link
Contributor

@amab8901 amab8901 commented Dec 5, 2022

Pull Request

Related issue

Fixes #3167 (meilisearch/meilisearch#3167)

What does this PR do?

  • allows Geosearch with zero radius to return the specified location when the coordinates match perfectly (instead of returning nothing). See link for more details.
  • new attempt on Geosearch for zero radius #713

PR checklist

Please check if your PR fulfills the following requirements:

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

Thank you so much for contributing to Meilisearch!

@irevoire irevoire self-requested a review December 5, 2022 19:32
@irevoire irevoire added bug Something isn't working no breaking The related changes are not breaking (DB nor API) labels Dec 5, 2022
milli/src/search/facet/filter.rs Outdated Show resolved Hide resolved
Copy link
Member

@irevoire irevoire left a comment

Choose a reason for hiding this comment

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

Thanks a lot @amab8901, I hope your next contribution will be easier 😂
bors merge

@amab8901
Copy link
Contributor Author

amab8901 commented Dec 5, 2022

Thanks a lot @amab8901, I hope your next contribution will be easier joy bors merge

haha I don't mind that it's hard. Getting stuck and then getting shown how to get unstuck is very educational and gives me valuable experience.

@irevoire
Copy link
Member

irevoire commented Dec 5, 2022

Also, just as a tip, when I contribute to an external repository, what I do is create two remotes.
One origin points to my fork, and one upstream points to the forked repository.

And then, when I want to make a new PR, I create a new branch with git checkout -b branch-name where I add my commits.
I open my PR from this branch.

And if I need to update my repo / keep it in sync with the forked repository, I come back on main, git checkout main and pull the latest changes: git pull upstream main.
Then I come back to my branch and run git rebase main.

Hope that helps with your future contribution! 🎉

@bors
Copy link
Contributor

bors bot commented Dec 5, 2022

@bors bors bot merged commit d6eacb2 into meilisearch:main Dec 5, 2022
Kerollmops pushed a commit that referenced this pull request Dec 6, 2022
722: Geosearch for zero radius r=irevoire a=amab8901

# Pull Request

## Related issue
Fixes #3167 (meilisearch/meilisearch#3167)

## What does this PR do?
- allows Geosearch with zero radius to return the specified location when the coordinates match perfectly (instead of returning nothing). See link for more details.
- new attempt on #713

## PR checklist
Please check if your PR fulfills the following requirements:
- [ X ] Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
- [ X ] Have you read the contributing guidelines?
- [ X ] Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!


Co-authored-by: amab8901 <amab8901@protonmail.com>
Co-authored-by: Tamo <irevoire@protonmail.ch>
bors bot added a commit that referenced this pull request Dec 6, 2022
725: Reapply fixes for v0.37.1 r=curquiza a=Kerollmops

This PR reapplies #712, #720, #722, and #723.
We needed the #720 as it was bringing more tests and snap-related improvements.

Co-authored-by: bors[bot] <26634292+bors[bot]@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working no breaking The related changes are not breaking (DB nor API)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants