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

Use prettier for style #49

Merged
merged 1 commit into from
Aug 26, 2017
Merged

Use prettier for style #49

merged 1 commit into from
Aug 26, 2017

Conversation

SimenB
Copy link
Collaborator

@SimenB SimenB commented Aug 22, 2017

This uses prettier through the eslint plugin to keep the workflow the same.

I removed the conflicting rules from eslintrc, and tried to keep the config they wanted (4 space indent, no semis and 100 char width).

@SimenB SimenB requested a review from af August 22, 2017 13:39
@af
Copy link
Owner

af commented Aug 23, 2017

Thanks for the PR! I'm a bit hesitant to merge it though, IMO prettier makes some sections (eg ternaries and some of the extra linebreaks) a little more difficult to read. I know it's being adopted widely, and I like the fact that it automates code style considerations, so I definitely see the value. Are there some more extra config options for the things I mentioned?

@SimenB
Copy link
Collaborator Author

SimenB commented Aug 23, 2017

No options for ternaries and extra linebreaks, no.

I know that doing something about ternaries is on the maintainers' list (at least nested ones https://github.com/prettier/prettier/pull/2398#issuecomment-315182247https://github.com/prettier/prettier/pull/2398#issuecomment-315182247).

WRT line breaks, prettier respects if you have extra line breaks in e.g. object literals. If there are any specific changes prettier did that you don't like, we can add a prettier-ignore comment above it

@af
Copy link
Owner

af commented Aug 23, 2017

Nice, I'll take this for a spin soon and see if prettier can win over this old-school manual formatting curmudgeon ;)

@kachkaev
Copy link
Contributor

It'd be great to see prettier in the project! I've been using it for a while and felt so odd while writing #55, because the code did not correct itself 😃

@SimenB could you please rebase this PR if #55 is merged?

@SimenB
Copy link
Collaborator Author

SimenB commented Aug 26, 2017

Yup, I keep rebasing it after every merge in the hopes of it getting merged 😄

@af
Copy link
Owner

af commented Aug 26, 2017

OK, I'm willing to give in on this one :) One more rebase from @SimenB and we can merge

@SimenB
Copy link
Collaborator Author

SimenB commented Aug 26, 2017

Rebased 🎉

@af af merged commit b971fea into af:master Aug 26, 2017
@SimenB SimenB deleted the prettier branch August 26, 2017 16:30
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