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

Format source using prettier #16

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

andrewiggins
Copy link
Collaborator

Hi @thlorenz 👋

I noticed in another PR there were some comments about formatting. I'm also looking at contributing to deoptigate and thought contributing may be easier if this repo used Prettier, since it appears you use it some of your other repos. Personally I use semis 😬 so it would at least automatically remove them for me if I accidentally add them.

I copied the prettier config from one of your other repos to hopefully maintain consistency with your preferred style. It looks like most of the changes are line-length changes and moving commas from the beginning of the line to the end of the line.

No pressure though if you think Prettier isn't right for this repo. Just thought I'd ask. I can always manually remove my semis in future PRs 😄

@thlorenz
Copy link
Owner

thlorenz commented Jul 4, 2020

In general I got no objection .. .let's merge the tests PR first though.

Also make sure that prettier + prettier fix each get a script in the package.json

@andrewiggins
Copy link
Collaborator Author

Yup, makes sense. You can use npm run lint and npm run lint:fix to run prettier -c and prettier --fix respectively.

@andrewiggins
Copy link
Collaborator Author

Rebased this PR on top of master to resolve conflicts in package.json, added npm run lint to the GitHub PR Action flow so PR builds fail if prettier wasn't run, and fixed the lint command to not lint the app/build directory.

I think this is ready for review/approval now!

@thlorenz
Copy link
Owner

Hey sorry for the late reply (again). Thanks for rebasing.
Did you ensure this still runs as before? If so I'll just merge it when once you confirm.

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.

2 participants