-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
feat(geo): support Language options in map searches #11178
feat(geo): support Language options in map searches #11178
Conversation
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. @@ Coverage Diff @@
## main #11178 +/- ##
==========================================
+ Coverage 83.23% 83.38% +0.14%
==========================================
Files 274 294 +20
Lines 20523 20862 +339
Branches 4436 4489 +53
==========================================
+ Hits 17083 17395 +312
- Misses 3152 3180 +28
+ Partials 288 287 -1 see 56 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Not sure which documentation to change. Much of it looks generated. And because of the line-numbers in the referencing docs some of it might need to be regenerate. |
…enbrand/amplify-js into Geo/feat/11177-Language
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.
The change looks good to me. Confirmed the new parameter is supported by underlying AWS SDK client. We just need to review the public interface change on Amplify library side.
.gitignore
Outdated
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.
I don't think changes in this file is necessary
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.
I think I reverted to the previous version of that file.
I keep updating the branch, but is there something I should still do on the PR? |
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.
LGTM!
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.
Looks good to me!
@roelvandenbrand, thank you for submitting this PR! We've gotten it merged and appreciate your patience and contributions. |
Description of changes
Added a optional language attribute to the Geo search mapper to be able to receive the response from AWS Location services in a desired language.
Issue #, if available
#11177
Description of how you validated changes
Added the optional language attribute to an existing test and verified it is in the request to the Location provider on AWS.
Ran "yarn test"
Checked failed tests, there were some on Cognito where nothing changed. No error in the Geo tests.
Checklist
yarn test
passes (for Geo component)By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.