-
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
chore: enable eslint for geo interactions and predictions packages #13092
Conversation
ec34905
to
5663c57
Compare
packages/geo/src/providers/location-service/AmazonLocationServiceProvider.ts
Show resolved
Hide resolved
5663c57
to
5c7e2b5
Compare
0df288d
to
990d3c1
Compare
return; | ||
} | ||
|
||
// Push all successes to results | ||
response.Successes?.forEach(success => { | ||
const { GeofenceId, CreateTime, UpdateTime } = success; | ||
const { GeofenceId: geofenceId, CreateTime, UpdateTime } = success; |
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.
For my curiosity, why was this needed for just GeofenceId
?
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.
GeofenceId
is declared in the upper scope already. shadow vars.
return; | ||
} | ||
|
||
const badGeofenceIds = response.Errors.map( | ||
const targetBadGeofenceIds = response.Errors.map( |
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.
Curious why this was needed too
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.
Ditto shadow vars
@@ -420,7 +430,7 @@ export class AmazonAIConvertPredictionsProvider { | |||
region: string; | |||
languageCode: string; | |||
}): Promise<WebSocket> { | |||
return new Promise(async (res, rej) => { | |||
return new Promise((resolve, _reject) => { |
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.
Nit: Drop the reject altogether?
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.
There is no await
in the promise executor function body. So the async
is unnecessary.
d43cc16
to
295ad16
Compare
295ad16
to
5809612
Compare
Description of changes
Details see each commit of this PR.
Issue #, if available
Description of how you validated changes
Checklist
yarn test
passesBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.