-
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(datastore): remove tslint and enable eslint #13316
Conversation
const result: T[] = await new Promise((resolve, reject) => { | ||
const result: T[] = await 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.
Not thrilled with this. With _
prefixed names indicating an unused variable, this just reads wrong. This feels like a consequence of the linter rule I complained about a little previously which prevents resolve, reject
from being named anything else.
Does it make sense to drop that rule?
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 see lots of other places the _
was also used for disambiguation between contexts. Not super thrilling. And, I'd still be interested in turning off the promise arg naming rule, personally. But, I don't want to block on this at all.
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.
We can discuss this and adjust the settings. Though this particular change was for the variable shadowing rule - that we had nested promise executors implementation.
ad3b7b3
to
7c1970c
Compare
…equries refactoring are suppresed by eslint ignore comment for stability)
7c1970c
to
43f6102
Compare
* chore(datastore): remove tslint and enable eslint * chore(datastore): run yarn lint:fix * chore(datastore): manual fix of linter reported issues (issues that requries refactoring are suppresed by eslint ignore comment for stability) * chore(data-storage-adapter): remove tslint and enable eslint * chore(data-storage-adapter): run yarn lint:fix * chore(data-storage-adapter): manual fix of linter reported issues * chore(repo): remove eslint muanl suppression
Description of changes
Details see commit messages in this PR.
E2E test run against the changes included in this PR: https://github.com/aws-amplify/amplify-js/actions/runs/8900079676
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.