Skip to content
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(storage): enable eslint and remove tslint #13004

Merged
merged 4 commits into from
Feb 16, 2024
Merged

Conversation

HuiSF
Copy link
Contributor

@HuiSF HuiSF commented Feb 12, 2024

Description of changes

See each commit message for details of changes.

Issue #, if available

Description of how you validated changes

Checklist

  • PR description included
  • yarn test passes
  • Tests are changed or added
  • Relevant documentation is changed or added (and PR referenced)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@HuiSF HuiSF requested a review from a team as a code owner February 12, 2024 22:54
stocaaro
stocaaro previously approved these changes Feb 13, 2024
.eslintrc.js Outdated
@@ -55,7 +55,7 @@ module.exports = {
'react-native',
'rtn-push-notification',
'rtn-web-browser',
'storage',
// 'storage',
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just delete this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Meant to delete everything et the end to avoid merge conflicts (if it really helped...)

Comment on lines +95 to +96
// TODO(eslint): remove this linter suppression.
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to address this in this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had the same question when this code is introduced...

There is no solution here as we can mutating the typescript thought "readonly" property. We can use as any here to remove the lint error though...

import { parseXmlError } from './utils';

const DOMAIN_PATTERN = /^[a-z0-9][a-z0-9\.\-]{1,61}[a-z0-9]$/;
const DOMAIN_PATTERN = /^[a-z0-9][a-z0-9.-]{1,61}[a-z0-9]$/;
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this a logic change from specifically a period to any character?

Copy link
Contributor Author

@HuiSF HuiSF Feb 14, 2024

Choose a reason for hiding this comment

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

Note that the . special char is in the character class [].

Inside a character class, the dot loses its special meaning and matches a literal dot.

So it matches the dot . specifically, not any single char, i.e. the escape slash is redundant (and \ cannot appear in a domain, so it's not the case trying to match a \ char either)

return;
}
// TODO(eslint): remove this linter suppression.
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto

ashika112
ashika112 previously approved these changes Feb 14, 2024
jimblanc
jimblanc previously approved these changes Feb 14, 2024
@HuiSF HuiSF dismissed stale reviews from jimblanc, ashika112, and stocaaro via cbb2ee8 February 15, 2024 17:08
@HuiSF HuiSF force-pushed the hui/chore/storage/eslint branch 3 times, most recently from 97bf5b4 to 6a92a05 Compare February 15, 2024 17:55
jimblanc
jimblanc previously approved these changes Feb 15, 2024
cshfang
cshfang previously approved these changes Feb 15, 2024
@HuiSF HuiSF dismissed stale reviews from cshfang and jimblanc via 7e72858 February 16, 2024 00:07
@HuiSF HuiSF force-pushed the hui/chore/storage/eslint branch 2 times, most recently from 7e72858 to 2e9c5d7 Compare February 16, 2024 17:26
@HuiSF HuiSF merged commit 0d2aa5d into main Feb 16, 2024
30 checks passed
@HuiSF HuiSF deleted the hui/chore/storage/eslint branch February 16, 2024 17:53
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.

None yet

5 participants