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

Adopt prettier #513

Merged
merged 1 commit into from
Aug 23, 2020
Merged

Adopt prettier #513

merged 1 commit into from
Aug 23, 2020

Conversation

SamChou19815
Copy link
Contributor

@SamChou19815 SamChou19815 commented Aug 23, 2020

Summary

The airbnb linter config introduces a lot of opinionated style lint rules. It helps us to establish a team wide style guide without fighting over styles, but it also adds significant burden to developers.

For example, a lot of style violations are not autofixable, so devs have to spend extra time to make linter happy. Some rules are even in conflict with each other sometimes, and devs need to find smart ways to work around them. What's worse, even after all these burdens, it's still not guaranteed that each AST corresponds to a unique style-guide approved output, so there can be still fighting over styles.

Therefore, I think it's better to introduce prettier for this case. Prettier formats all the code in an opinionated way, and it can be run in a pre-commit hook. Therefore, even if you code style looks like garbage, the pre-commit hook will fix it automatically for you like:

Screen Shot 2020-08-23 at 18 16 45

The diff is big, but almost all the changes are generated by yarn format.

Test Plan

CI, and use you 👀 to decide whether prettier produces nice output.

@dti-github-bot
Copy link
Member

dti-github-bot commented Aug 23, 2020

[diff-counting] Significant lines: 1839. This diff might be too big! Developer leads are invited to review the code.

@dti-github-bot dti-github-bot requested review from lsizemore8, dti-github-bot and JBoss925 and removed request for lsizemore8 August 23, 2020 22:18
@dti-github-bot
Copy link
Member

[deployment-bot] Deployed to https://staging-pr.samwise.today/513/index.html

@dti-github-bot dti-github-bot requested review from lsizemore8 and removed request for lsizemore8 August 23, 2020 22:24
@SamChou19815 SamChou19815 marked this pull request as ready for review August 23, 2020 22:28
@SamChou19815 SamChou19815 requested a review from a team as a code owner August 23, 2020 22:28
Copy link
Member

@ptwu ptwu left a comment

Choose a reason for hiding this comment

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

Sg. Do you have a particular prettierrc file that you recommend we use? (other than just enable singlequote)

@SamChou19815
Copy link
Contributor Author

SamChou19815 commented Aug 23, 2020

Sg. Do you have a particular prettierrc file that you recommend we use? (other than just enable singlequote)

I put the config inside the package.json, so we don't need another dotfile in the root directory

Copy link
Member

@meganyin13 meganyin13 left a comment

Choose a reason for hiding this comment

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

@SamChou19815 SamChou19815 merged commit 0c194cd into master Aug 23, 2020
@SamChou19815 SamChou19815 deleted the adopt-prettier branch August 23, 2020 23:51
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.

4 participants