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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve eslint config #2042

Merged
merged 11 commits into from Apr 11, 2019
Merged

Improve eslint config #2042

merged 11 commits into from Apr 11, 2019

Conversation

amio
Copy link
Contributor

@amio amio commented Mar 30, 2019

  • Get .ts files covered 馃帀
  • Update eslint related dependencies
  • Add typescript-eslint recommended rules, but only enabled ones we already followed, commented ones causing errors.

Commented rules including:

  # '@typescript-eslint/no-unused-vars': 1
  # '@typescript-eslint/indent': ['error', 2]
  # '@typescript-eslint/array-type': error
  # '@typescript-eslint/ban-types': error
  # '@typescript-eslint/explicit-member-accessibility': error
  # '@typescript-eslint/member-delimiter-style': error
  # '@typescript-eslint/no-angle-bracket-type-assertion': error
  # '@typescript-eslint/no-explicit-any': warn
  # '@typescript-eslint/no-object-literal-type-assertion': error
  # '@typescript-eslint/no-use-before-define': error
  # '@typescript-eslint/no-var-requires': error
  # '@typescript-eslint/prefer-interface': error

TODO

Go through these commented rules one by one, enable & fix for it or confirm it's disabled. Since enabling some rules would cause massive code change (like '@typescript-eslint/indent': ['error', 2]), which might conflict with our ongoing development, we better do this in separated PRs.

- Add `.ts` extension
- Update dependencies related to eslint
- Disabled two problemtic ruls temporally
@codecov-io
Copy link

codecov-io commented Apr 9, 2019

Codecov Report

Merging #2042 into canary will increase coverage by <.01%.
The diff coverage is 3.84%.

Impacted file tree graph

@@            Coverage Diff            @@
##           canary   #2042      +/-   ##
=========================================
+ Coverage    4.39%   4.39%   +<.01%     
=========================================
  Files         243     243              
  Lines        8694    8693       -1     
  Branches      904     904              
=========================================
  Hits          382     382              
+ Misses       8295    8294       -1     
  Partials       17      17
Impacted Files Coverage 螖
src/commands/deploy/legacy.js 0% <酶> (酶) 猬嗭笍
src/util/dns/import-zonefile.ts 0% <酶> (酶) 猬嗭笍
src/util/read-metadata.js 100% <酶> (酶) 猬嗭笍
src/util/alias/deployment-should-downscale.ts 0% <酶> (酶) 猬嗭笍
src/util/config/files.ts 0% <酶> (酶) 猬嗭笍
src/util/config/get-default.js 0% <酶> (酶) 猬嗭笍
src/util/alias/validate-path-alias-rules.ts 0% <酶> (酶) 猬嗭笍
src/commands/init/init.ts 0% <酶> (酶) 猬嗭笍
src/util/error-handlers.ts 0% <酶> (酶) 猬嗭笍
src/util/domains/check-transfer.ts 0% <酶> (酶) 猬嗭笍
... and 31 more

Continue to review full report at Codecov.

Legend - Click here to learn more
螖 = absolute <relative> (impact), 酶 = not affected, ? = missing data
Powered by Codecov. Last update ff353a2...93e0a68. Read the comment docs.

@amio amio marked this pull request as ready for review April 9, 2019 05:06
Copy link
Member

@styfle styfle left a comment

Choose a reason for hiding this comment

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

Looks ok. Let's get someone else to approve too 馃憤

@amio
Copy link
Contributor Author

amio commented Apr 10, 2019

Ping @leo @TooTallNate @javivelasco, could you help review this? 馃檹

.eslintrc.yml Outdated Show resolved Hide resolved
@amio amio merged commit 96c630b into canary Apr 11, 2019
@amio amio deleted the fix/eslint-config branch April 11, 2019 09:30
leo pushed a commit that referenced this pull request Apr 11, 2019
- Get `.ts` files covered 馃帀
- Update eslint related dependencies
- Add typescript-eslint [recommended rules](https://github.com/typescript-eslint/typescript-eslint/blob/3e26ab684ae5642e9c50940bedd155e32e189900/packages/eslint-plugin/src/configs/recommended.json), but only enabled ones we already followed, commented ones causing errors.

Commented rules including:

```yaml
  # '@typescript-eslint/no-unused-vars': 1
  # '@typescript-eslint/indent': ['error', 2]
  # '@typescript-eslint/array-type': error
  # '@typescript-eslint/ban-types': error
  # '@typescript-eslint/explicit-member-accessibility': error
  # '@typescript-eslint/member-delimiter-style': error
  # '@typescript-eslint/no-angle-bracket-type-assertion': error
  # '@typescript-eslint/no-explicit-any': warn
  # '@typescript-eslint/no-object-literal-type-assertion': error
  # '@typescript-eslint/no-use-before-define': error
  # '@typescript-eslint/no-var-requires': error
  # '@typescript-eslint/prefer-interface': error
```

### TODO

Go through these commented rules one by one, enable & fix for it or confirm it's disabled. Since enabling some rules would cause massive code change (like `'@typescript-eslint/indent': ['error', 2]`), which might conflict with our ongoing development, we better do this in separated PRs.
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

4 participants