-
Notifications
You must be signed in to change notification settings - Fork 10
[risk=no][RW-7280] Enable some Prettier-compatible eslint rules #6199
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
Conversation
'prettier', | ||
'react', | ||
'react-hooks', | ||
'simple-import-sort', |
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.
remove for now; I have a speculative follow-on PR to re-enable.
'react', | ||
'react-hooks', | ||
'simple-import-sort', | ||
'sort-keys-fix', |
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.
whatever this was, we no longer refer to it
'@typescript-eslint/prefer-optional-chain': 'warn', | ||
'@typescript-eslint/unified-signatures': 'warn', | ||
// '@typescript-eslint/member-ordering': ['warn', { 'classExpressions': ['method', 'field'] }], | ||
// '@typescript-eslint/prefer-function-type': 'warn', |
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 judged these to be more "functional" than "style" so I moved them to this section
|
||
'prettier/prettier': 'warn', | ||
|
||
'max-len': [ |
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.
These 2 are compatible and non-redundant to Prettier
|
||
// temp disable others. re-enable if desired after determining that they don't conflict with prettier. | ||
|
||
// 'eol-last': 'warn', |
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.
redundant
// 'eol-last': 'warn', | ||
// 'max-len': ['warn', {code: 140, ignorePattern: '^import |^export\\{(.*?)\\}', ignoreComments: true}], | ||
// 'no-trailing-spaces': 'warn', | ||
// 'quotes': ['warn', 'single'], |
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.
redundant
// 'max-len': ['warn', {code: 140, ignorePattern: '^import |^export\\{(.*?)\\}', ignoreComments: true}], | ||
// 'no-trailing-spaces': 'warn', | ||
// 'quotes': ['warn', 'single'], | ||
// // 'brace-style': ['warn', '1tbs'], |
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.
redundant
// 'no-trailing-spaces': 'warn', | ||
// 'quotes': ['warn', 'single'], | ||
// // 'brace-style': ['warn', '1tbs'], | ||
// // 'no-multi-spaces': 'warn', |
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.
redundant
// // '@typescript-eslint/semi': 'warn', | ||
// | ||
// 'react/jsx-curly-spacing': ["warn", {'when': 'never'}], | ||
// 'simple-import-sort/sort': 'warn', |
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.
deferred- these 2 are bigger changes, and maybe not desirable.
// // 'brace-style': ['warn', '1tbs'], | ||
// // 'no-multi-spaces': 'warn', | ||
// // 'simple-import-sort/sort': 'warn', | ||
// // 'space-before-function-paren': ['warn', { 'anonymous': 'never', 'named': 'never', 'asyncArrow': 'always' }], |
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.
incompatible
// // 'space-before-function-paren': ['warn', { 'anonymous': 'never', 'named': 'never', 'asyncArrow': 'always' }], | ||
// // 'spaced-comment': 'warn', | ||
// | ||
// '@typescript-eslint/type-annotation-spacing': 'warn', |
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.
redundant
// '@typescript-eslint/prefer-function-type': 'warn', | ||
// // '@typescript-eslint/semi': 'warn', | ||
// | ||
// 'react/jsx-curly-spacing': ["warn", {'when': 'never'}], |
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.
redundant
if (enablePd) { | ||
return; | ||
it( | ||
'should set runtime preset values in customize panel instead of getRuntime values ' + |
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.
line was too long
); | ||
|
||
it( | ||
'should set runtime preset values in customize panel instead of getRuntime values ' + |
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.
line was too long
CdrVersionsStubVariables.DEFAULT_WORKSPACE_CDR_VERSION_ID | ||
); | ||
|
||
const expectedUpgradeMessage = `${CdrVersionsStubVariables.ALT_WORKSPACE_CDR_VERSION} to ${CdrVersionsStubVariables.DEFAULT_WORKSPACE_CDR_VERSION}.`; |
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.
line too long
}); | ||
|
||
it('should return resource URLs', () => { | ||
const WORKSPACE_URL_PREFIX = `/workspaces/${WorkspaceStubVariables.DEFAULT_WORKSPACE_NS}/${WorkspaceStubVariables.DEFAULT_WORKSPACE_ID}`; |
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.
line too long
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.
thanks
Some rules are compatible with Prettier, some are incompatible, and some are redundant to it.
PR checklist
yarn test-local
or yarn test-local-devup