Skip to content

chore: format code with semicolons when using prettier #9555

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

Merged
merged 1 commit into from
Sep 6, 2023
Merged

Conversation

aslilac
Copy link
Member

@aslilac aslilac commented Sep 6, 2023

  • It's not common, but JavaScript's semi-colon insertion rules can be painful sometimes
  • The ecosystem has largely grown to favor semicolons in recent years
  • ...which can largely be attributed to Prettier, which formats code using semicolons by default
  • Sticking closer to the defaults of Prettier makes us a good citizen of the JavaScript-kingdom

Instead of reviewing this by reading through the diff, I might suggest reviewing by reproducing.

  • Make sure your local refs are up-to-date
    git fetch origin
    
  • Checkout the latest commit from the main branch
    git checkout b15bfa4
    
  • Manually set your HEAD to this branch
    git reset origin/semicolons
    

You should now have a huge git diff of 664 files. Your HEAD includes these changes, but your local files will still be the same as on the main branch.

  • Edit .prettierrc.yaml
    - semi: false
  • Regenerate site/.prettierrc.yaml
    make site/.prettierrc.yaml
    
  • Format all of the code!
    make fmt
    

If you check git diff now, you should see that only one other small difference remains (I added a space to a comment)

@aslilac
Copy link
Member Author

aslilac commented Sep 6, 2023

We can also avoid having this completely ruin git blame by creating a .git-blame-ignore-revs file that contains the hash of the squashed commit once we merge this. Since we need to know the hash first, that will have to be done as a separate PR.

Copy link
Collaborator

@BrunoQuaresma BrunoQuaresma left a comment

Choose a reason for hiding this comment

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

If CI is happy, I'm good with it. Just missing the reasoning behind these changes in the PR description.

@BrunoQuaresma
Copy link
Collaborator

We can also avoid having this completely ruin git blame by creating a .git-blame-ignore-revs file that contains the hash of the squashed commit once we merge this. Since we need to know the hash first, that will have to be done as a separate PR.

TIL

@aslilac aslilac changed the title chore: use semicolons in js/ts chore: format code with semicolons when using prettier Sep 6, 2023
@aslilac aslilac merged commit 988c9af into main Sep 6, 2023
@aslilac aslilac deleted the semicolons branch September 6, 2023 18:59
@github-actions github-actions bot locked and limited conversation to collaborators Sep 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants