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: Upgrade eslint config #875

Closed
wants to merge 2 commits into from

Conversation

G2Jose
Copy link
Contributor

@G2Jose G2Jose commented May 5, 2019

Description

Changes

Updated the following packages related to linting:

  • eslint
  • babel-eslint

Adds the following packages:

  • prettier-eslint

Reformat files that do not comply with linting configuration.

  • The main change is the later version of eslint applies bracket spacing rules to async functions too - Example here.

Rename .eslintrc -> .eslintrc.js so it can be automatically formatted too.

Motivation


Question Response
Version? v1.4.1
Devices tested? N/A
Bug fix? no
New feature? no
Includes tests? no
All Tests pass? yes
Related ticket? N/A

G2Jose added 2 commits May 5, 2019 12:48
Move to prettier-eslint in lint-staged. Upgrade eslint, babel-eslint dependencies. Rename .eslintrc
-> .eslintrc.js
Reformat files using updated eslint, babel-eslint, prettier-eslint
@coveralls
Copy link

Coverage Status

Coverage remained the same at 48.895% when pulling c314b18 on G2Jose:upgrade-eslint-config into 9af60b2 on gitpoint:master.

@G2Jose G2Jose changed the title Upgrade eslint config chore: Upgrade eslint config May 5, 2019
Copy link
Member

@chinesedfan chinesedfan left a comment

Choose a reason for hiding this comment

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

Good enhancement. And I have few questions.

Rename .eslintrc -> .eslintrc.js so it can be automatically formatted too.

Is it depend on your IDE? I don't know why .eslintrc.js can work, but .eslintrc doesn't.


Edit: I found .eslintrc was deprecated, https://dev.to/ohbarye/eslintrc-without-file-extension-is-deprecated-3ngg. Is it the reason?

<CommitListItem commit={item} navigation={this.props.navigation} />;
renderItem = ({ item }) => (
<CommitListItem commit={item} navigation={this.props.navigation} />
);
Copy link
Member

Choose a reason for hiding this comment

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

Can we do some prettier/eslint configurations so that these changes will not be made? As well as the space after async and extra empty line between every let/const.

@chinesedfan
Copy link
Member

Updates,

-    'newline-after-var': ['error', 'always'],
-    'newline-before-return': 'error',
+    'padding-line-between-statements': [
+        2,
+        { 'blankLine': 'always', 'prev': '*', 'next': 'return' },
+        { 'blankLine': 'always', 'prev': ['var', 'let', 'const'], 'next': '*' },
+        { 'blankLine': 'any', 'prev': ['var', 'let', 'const'], 'next': ['var', 'let', 'const'] },
+    ],
     'newline-per-chained-call': 'off',
     'no-confusing-arrow': 'off',
     'no-mixed-operators': [
@@ -71,7 +75,14 @@ module.exports = {
         allowForLoopAfterthoughts: true,
       },
     ],
-    'space-before-function-paren': ['error', 'never'],
+    'space-before-function-paren': [
+      'error',
+      {
+        anonymous: 'never',
+        named: 'never',
+        asyncArrow: 'always'
+      }
+    ],

@chinesedfan
Copy link
Member

prettier-eslint-cli@5 released today, with eslint@5.

@G2Jose Can you update this PR or would you mind me taking over it?

@chinesedfan
Copy link
Member

Close in favor of #882. Still many thanks to @G2Jose.

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.

3 participants