Skip to content
This repository has been archived by the owner on Sep 18, 2024. It is now read-only.

[webui] format code by eslint and prettier #2744

Merged
merged 13 commits into from
Sep 18, 2020

Conversation

Lijiaoa
Copy link
Contributor

@Lijiaoa Lijiaoa commented Jul 28, 2020

  • stylelint reference

    • yarn stylelint --fix could almost format css/scss code
  • prettier

  • add some eslint rule

 // no use `arrow-parens: [2, "as-needed", { "requireForBlockBody": true }]`,
 // because `"requireForBlockBody": true`  is conflict with prettier `arrowParens: 'avoid',`
 "arrow-parens": [2, "as-needed"],
 "no-empty": 2, // block is null will error
 "no-multiple-empty-lines": [2, { "max": 1 }], // max empty line is 1

@@ -41,7 +47,7 @@ class ChangeColumnComponent extends React.Component<ChangeColumnProps, ChangeCol
} else {
if (source.includes(label)) {
// remove from source
const result = source.filter((item) => item !== label);
const result = source.filter(item => item !== label);
Copy link
Contributor

@liuzhe-lz liuzhe-lz Aug 10, 2020

Choose a reason for hiding this comment

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

I think this is less clear to human. It's hard to figure out associative order at first glance to me.
I personally prefer item => (item !== label), but according to line 44 of App.tsx maybe tslint doesn't like it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

alwaysParens: avoid | always;
default val is avoid, not add () for arrow function.
So we change it to always?

@@ -0,0 +1,12 @@
root = true
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this file used by a specific text editor? If yes it should not be committed to code tree.

end_of_line = lf
charset = utf-8
trim_trailing_whitespace = true
insert_final_newline = true
Copy link
Contributor

@liuzhe-lz liuzhe-lz Aug 12, 2020

Choose a reason for hiding this comment

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

And I wonder why your files do not have trailing new line even though this is set to true...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but .tsx .scss these files all have new line in last line.

item.errorWhere && <div key={key} className="warning">
<MessageInfo info={item.errorMessage} typeInfo="error" />
{errorList.map((item, key) => {
return (
Copy link
Contributor

Choose a reason for hiding this comment

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

Is return necessary here?

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 also think so. We can use arrow function rules to standardize it.

@liuzhe-lz
Copy link
Contributor

liuzhe-lz commented Aug 12, 2020

I think we should discuss the rules in a meeting, just like what we have done for pylint rules.
For pylint, we listed all default rules violated by our code, and discussed them one by one in a meeting to decide if the rule should be enabled.

@scarlett2018 scarlett2018 mentioned this pull request Sep 11, 2020
79 tasks
@Lijiaoa Lijiaoa closed this Sep 11, 2020
@Lijiaoa Lijiaoa reopened this Sep 11, 2020
@Lijiaoa Lijiaoa closed this Sep 14, 2020
@Lijiaoa Lijiaoa reopened this Sep 14, 2020
@Lijiaoa Lijiaoa closed this Sep 16, 2020
@Lijiaoa Lijiaoa reopened this Sep 16, 2020
@Lijiaoa Lijiaoa closed this Sep 17, 2020
@Lijiaoa Lijiaoa reopened this Sep 17, 2020
@ultmaster ultmaster merged commit e2d8cc1 into microsoft:master Sep 18, 2020
@Lijiaoa Lijiaoa mentioned this pull request Oct 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants