Skip to content

Adds prettier via eslint #94

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

Closed
wants to merge 2 commits into from
Closed

Conversation

deini
Copy link
Member

@deini deini commented Apr 24, 2019

Adds prettier via eslint using the same config as v1 trunk. @arcanis I had to delete a couple of your arca rules to make it compatible with prettier.

I went with a .prettierrc instead of adding the prettier rules inside of .eslintrc.json for better editor support.

The actual setup recommendation for eslint-plugin-prettier is to also include eslint-config-prettier which disables conflicting rules. Since the current eslint config is pretty basic I don't see the need right now, but might be useful in the future? 🤷‍♂️ Let me know if you guys would prefer that and I can include it.

There is also the option to run prettier directly from its cli instead of through eslint but IMO it is nicer to run just one thing, have the error format, etc.

Closes #12

@deini deini force-pushed the prettierViaEslint branch from a373020 to 319fc7d Compare April 24, 2019 22:55
@arcanis
Copy link
Member

arcanis commented Apr 25, 2019

Hey! Thanks for doing this! I however have some concerns, I hope you won't mind me sharing them.

Since the last time we mentioned the subject over at #12 my view slightly changed - I still strongly believe we should have automatic formatting to lower the contribution bar, but I'm not convinced anymore Prettier is the solution I'd advocate for.

The problem is while Prettier does have good features, it also makes questionable choices in term of readability1 and sometimes lead to strictly worse code. It does avoid some amount of bikeshedding (and still here we are by my fault 😅), but in our case I think I'll make the calls anyway for the time being so that shouldn't be a problem. Remains the automatic formatting.

What I'd like to do is see whether we can solve the user story ("I'm a contributor and I don't want to think twice about the way I write my code") in a more flexible way through fixable ESLint plugins (cf for example #70). The workflow would be the same but we would have some control on what we want to enable or not and the authors would keep some liberty to format their code differently depending on what they want to highlight (always in the boundaries of the rules we set).

If a style sounds off but the linter doesn't warn then reviewers will be tasked from doing the changes themselves and it won't be held against the initial PR author (it's much easier now that Github allows suggested changes from the interface).

We can experiment this for let's say six months and revisit if needed. What do you say? Does that sound reasonable?


1 at least for me those compressed one-line for statements are unreadable

@deini
Copy link
Member Author

deini commented Apr 29, 2019

Absolutely, let's wait for six months and revisit if needed 😄. I'm happy with eslint fixers, it's more work to create custom ones but having the control of the output sounds like the best fit for the project.

@deini deini closed this Apr 29, 2019
@arcanis arcanis mentioned this pull request May 7, 2019
1 task
@paul-soporan paul-soporan mentioned this pull request Jun 4, 2020
3 tasks
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.

Consider using Prettier
2 participants