Skip to content

Conversation

@guimachiavelli
Copy link
Member

@guimachiavelli guimachiavelli commented Jun 16, 2022

Closes #1694

@guimachiavelli guimachiavelli changed the base branch from master to v0.28 June 16, 2022 09:47
@guimachiavelli guimachiavelli marked this pull request as ready for review June 16, 2022 09:48
Copy link
Contributor

@maryamsulemani97 maryamsulemani97 left a comment

Choose a reason for hiding this comment

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

I can't comment on lines 132 and 219, but we need to update them as well

@guimachiavelli guimachiavelli added this to the v0.28 milestone Jun 20, 2022
@guimachiavelli
Copy link
Member Author

I can't comment on lines 132 and 219, but we need to update them as well

I initially thought the same thing, but after looking at it a bit more closely I changed my mind. Both _geoPoint and _geoRadius must be used as a value for a search parameter (sort and filter, respectively). This means both are always sent as strings, no? E.g. { "filter": "_geoRadius(45.472735, 9.184019, 2000)" }.

We could argue that we shouldn't refer to these as floating point numbers, but I think the intent is clear enough. If you think it's better to change it anyway, I guess we could replace "two floating point numbers indicating" with "two values indicating".

Copy link
Contributor

@maryamsulemani97 maryamsulemani97 left a comment

Choose a reason for hiding this comment

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

🥐

@maryamsulemani97
Copy link
Contributor

bors merge

@bors
Copy link
Contributor

bors bot commented Jun 29, 2022

Build succeeded:

@bors bors bot merged commit 13459d7 into v0.28 Jun 29, 2022
@bors bors bot deleted the v0.28-geo-values branch June 29, 2022 10:29
@guimachiavelli guimachiavelli mentioned this pull request Jun 30, 2022
17 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

v0.28: _geo accepts string values

2 participants