-
Notifications
You must be signed in to change notification settings - Fork 76
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
Go: Add command GeoSearch
#3385
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: TJ Zhang <[email protected]>
Signed-off-by: TJ Zhang <[email protected]>
Signed-off-by: TJ Zhang <[email protected]>
Signed-off-by: TJ Zhang <[email protected]>
searchFromArgs, err := searchFrom.ToArgs() | ||
if err != nil { | ||
return nil, err | ||
} | ||
args = append(args, searchFromArgs...) | ||
searchByShapeArgs, err := searchByShape.ToArgs() | ||
if err != nil { | ||
return nil, err | ||
} | ||
args = append(args, searchByShapeArgs...) | ||
infoOptionsArgs, err := infoOptions.ToArgs() | ||
if err != nil { | ||
return nil, err | ||
} | ||
args = append(args, infoOptionsArgs...) | ||
resultOptionsArgs, err := resultOptions.ToArgs() | ||
if err != nil { | ||
return nil, err | ||
} | ||
args = append(args, resultOptionsArgs...) | ||
result, err := client.executeCommand(C.GeoSearch, args) | ||
if err != nil { | ||
return nil, err | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could probably move this common part to another method
// | ||
// Parameters: | ||
// | ||
// key - The key of the sorted set. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh why such big idents?
searchFrom options.GeoSearchOrigin, | ||
searchByShape options.GeoSearchShape, | ||
infoOptions options.GeoSearchInfoOptions, | ||
) ([]any, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we create a custom return type there?
// The unit of measurement for the geospatial data | ||
type GeoUnit string | ||
|
||
const ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why there?
ToArgs() ([]string, error) | ||
} | ||
|
||
const ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move to const?
// The shape of the search area for the `GeoSearch` command | ||
type SearchShape string | ||
|
||
const ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and this
Issue link
This Pull Request is linked to issue (URL): [REPLACE ME]
Checklist
Before submitting the PR make sure the following are checked: