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

Eslint/Prettier Configuration Proposal #139

Closed
wants to merge 2 commits into from
Closed

Conversation

rori4
Copy link
Contributor

@rori4 rori4 commented Aug 19, 2019

I have created an example configuration for eslit/prettier to be integrated with the project. The configuration is set only through the .eslintrc and uses prettier for formatting.

Why use prettier with eslint?
Eslint does a poor job in autofix / autoformat of code style. The following styles give only warnings from eslint => max-len, no-mixed-spaces-and-tabs, keyword-spacing, comma-style...
https://prettier.io/docs/en/comparison.html
prettier/prettier-eslint#101 (comment)

Also I moved the eslint config dependencies to dev-dependencies and added the following scripts:

  • "lint": "eslint src/**/*.{js,jsx} && echo Linting check finished",
    
  • "lint:fix": "eslint --fix src/**/*.{js,jsx} && echo Linting fix finished",
    
  • "eslint-check": "eslint --print-config src/App.js | eslint-config-prettier-check"
    

the lint script will check the src folder with the linter and the lint:fix will autofix / format with prettier and eslint the code when possible.

the eslint-check script will show if there are settings conflicts between prettier and eslint. Style configurations should be set with prettier here as they might cause conflict. for example "indent": ["error", "tab"], is one setting that might cause conflict if set with eslint. Running this script will show the conflicts and they can easily be removed

We can also use pretty-quick library to do linting always before commits. This way we will always have consistently styled code.

Let me know what you think.

@rori4 rori4 requested a review from ivopaunov August 19, 2019 12:16
Copy link
Member

@ivopaunov ivopaunov left a comment

Choose a reason for hiding this comment

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

OK, thats great.
As I told you, I want to merge counterfactual-accounts first before doing this.
I'll try to merge this into the the other branch and if there are no conflicts I'll merge this into master.

@ivopaunov
Copy link
Member

Can you push hust the updated package.json, eslintrc.json, and package-lock.json.
I'll try to merged it this way because now I have tons of conflicts.

@rori4 rori4 closed this Aug 19, 2019
@ivopaunov
Copy link
Member

#140

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

2 participants